| Message ID | cover.1612998106.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | midx: implement a multi-pack reverse index | expand |
On 2/10/21 6:02 PM, Taylor Blau wrote: > This series describes and implements a reverse index for the multi-pack index, > based on a "pseudo-pack" which can be uniquely described by the multi-pack > index. > > The details of the pseudo-pack, and multi-pack reverse index are laid out in > detail in the sixth patch. > > This is in support of multi-pack reachability bitmaps, which contain objects > from the multi-pack index. Likewise, an object's bit position in a multi-pack > reachability bitmap is determined by its position with that multi-pack index's > pseudo pack. This has been a lot of work, but I'm impressed with the progress here. This series is good prep, and my comments are very minor. Since the need for these multi-pack-index-<hash>.rev files doesn't show up until the reachability bitmaps can be paired with MIDX files, it would make sense to hold this series in 'next' until that one also stabilizes and they merge to 'master' together. Thanks, -Stolee
On Wed, Feb 10, 2021 at 09:58:18PM -0500, Derrick Stolee wrote: > This series is good prep, and my comments are very minor. Thanks for your review, especially on a series like this one that adds lots of code without any users :). > Since the need for these multi-pack-index-<hash>.rev files doesn't show > up until the reachability bitmaps can be paired with MIDX files, it > would make sense to hold this series in 'next' until that one also > stabilizes and they merge to 'master' together. That's fine with me. It would be OK to merge this down to 'master', too, since this is all dead code. In fact, that may be easier to work with, since the next topic can be based directly off 'master' instead of having to keep this branch around forever. Either is fine, though. Thanks, Taylor
Taylor Blau <me@ttaylorr.com> writes: > Since tb/pack-revindex-on-disk is queued to be merged to 'master', but hasn't > yet been merged, this series is based on that branch. This seems to have a light conflict with Derrick's chunked file format work in midx.c where pack_info is renamed and extended so the new pack_order variable now needs to become a member in it. I think I resolved it OK, but without any callers that actually utilize the new code or tests, it is almost impossible to have any confidence in the result of the conflict resolution X-<. Could you two please look over to see if I made any silly mistakes, when I pushe it out later? Thanks.
On 2/11/2021 3:13 AM, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > >> Since tb/pack-revindex-on-disk is queued to be merged to 'master', but hasn't >> yet been merged, this series is based on that branch. > > This seems to have a light conflict with Derrick's chunked file > format work in midx.c where pack_info is renamed and extended so the > new pack_order variable now needs to become a member in it. > > I think I resolved it OK, but without any callers that actually > utilize the new code or tests, it is almost impossible to have any > confidence in the result of the conflict resolution X-<. > > Could you two please look over to see if I made any silly mistakes, > when I pushe it out later? I reproduced your merge and got very similar results. The differences that happened between my result and yours are not meaningful in any way. Definitely a very subtle merge, so thank you for doing that! -Stolee
Derrick Stolee <stolee@gmail.com> writes: >> Could you two please look over to see if I made any silly mistakes, >> when I pushe it out later? > > I reproduced your merge and got very similar results. The differences > that happened between my result and yours are not meaningful in any > way. > > Definitely a very subtle merge, so thank you for doing that! Resolving this kind of conflict is like a quick quiz in the classroom to check my understanding of what is happening in all the topics involved. It is like solving a puzzle, and I do not dislike doing it (otherwise I won't be working as a maintainer), but having a second set of eyes, helping with independent verification, is always appreciated. Thanks.