Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote: > Here is an attempt to reword this a bit: > > The bitmap lookup table extension was documented by an earlier > change, but Git does not yet know how to parse that information. > The extension allows parsing a smaller portion of the bitmap > file in order to find bitmaps for specific commits. Got it. Thanks. > This environment variable does not appear to be used or > documented anywhere. Do we really want to use it as a way > to disable reading the lookup table in general? Or would it be > better to have a GIT_TEST_* variable for disabling the read > during testing? GIT_TEST_* is perfect. This was mainly for testing purpose. > Here, uint32_T is probably fine, but maybe we should just use size_t instead? Should we use st_mult() and st_add() everywhere? Yeah, it would be better to use st_*(). > I see that we have a two-method recursion loop. Please move this > declaration to immediately before lazy_bitmap_for_commit() so it > is declared as late as possible. Ok. > These two helpers should probably return a size_t and uint32_t > instead of a pointer. Let these do get_be[32|64]() on the computed > pointer. Ok. > This is using an interesting type of tail-recursion. We might be > better off using a loop with a stack: push to the stack the commit > positions of the XOR bitmaps. At the very bottom, we get a bitmap > without an XOR base. Then, pop off the stack, modifying the bitmap > with XOR operations as we go. (Perhaps we also store these bitmaps > in-memory along the way?) Finally, we have the necessary bitmap. Hmm, got the point. I need to fix the xor-offset related issue first (That I said earlier) before doing this. > Perhaps it is time to use hexadecimal representation here to match the > file format document? Yeah, of course! Thanks :)
