Re: [PATCH v4 07/13] pack-bitmap.c: teach `rev-list --test-bitmap` about incremental MIDXs
- From
Elijah Newren <newren@gmail.com>
- Date
- Mar 18, 2025, 05:31 UTC
- Message-ID
- <CABPp-BEyKkbVMs=7AgSf1P9CTXqA5CY5epG-piuSzppAWfPe4g@mail.gmail.com>
- In-Reply-To
- <b45a9ccbc20180e3358e314b4fd5e46bfa566241.1741983492.git.me@ttaylorr.com>
On Fri, Mar 14, 2025 at 1:18 PM Taylor Blau <me@ttaylorr.com> wrote:
Show 27 quoted lines
> > Implement support for the special `--test-bitmap` mode of `git rev-list` > when using incremental MIDXs. > > The bitmap_test_data structure is extended to contain a "base" pointer > that mirrors the structure of the bitmap chain that it is being used to > test. > > When we find a commit to test, we first chase down the ->base pointer to > find the appropriate bitmap_test_data for the bitmap layer that the > given commit is contained within, and then perform the test on that > bitmap. > > In order to implement this, light modifications are made to > bitmap_for_commit() to reimplement it in terms of a new function, > find_bitmap_for_commit(), which fills out a pointer which indicates the > bitmap layer which contains the given commit. > > Signed-off-by: Taylor Blau <me@ttaylorr.com> > --- > pack-bitmap.c | 107 ++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 86 insertions(+), 21 deletions(-) > > diff --git a/pack-bitmap.c b/pack-bitmap.c > index 7a41535425..bb09ce3cf5 100644 > --- a/pack-bitmap.c > +++ b/pack-bitmap.c
[...]
Show 5 quoted lines
> +
> +static void bitmap_test_data_prepare(struct bitmap_test_data *tdata,
> + struct bitmap_index *bitmap_git)
> +{
> + memset(tdata, 0, sizeof(struct bitmap_test_data));So, the first thing this function does is 0 out tdata.
Show 10 quoted lines
> +
> + tdata->bitmap_git = bitmap_git;
> + tdata->base = bitmap_new();
> + tdata->commits = ewah_to_bitmap(bitmap_git->commits);
> + tdata->trees = ewah_to_bitmap(bitmap_git->trees);
> + tdata->blobs = ewah_to_bitmap(bitmap_git->blobs);
> + tdata->tags = ewah_to_bitmap(bitmap_git->tags);
> +
> + if (bitmap_git->base) {
> + CALLOC_ARRAY(tdata->base_tdata, 1);We use CALLOC to both allocate the array and set it all to 0...
> + bitmap_test_data_prepare(tdata->base_tdata, bitmap_git->base);
and then call bitmap_test_data_prepare() which will re-zero it all out.
Should we either ditch the zeroing at the beginning of the function, or use xmalloc instead of CALLOC_ARRAY, to avoid duplicate zeroing?
> + } > +}
Didn't spot anything else.