On Fri, Mar 14, 2025 at 04:18:53PM -0400, Taylor Blau wrote:
> @@ -81,6 +81,24 @@ struct bitmap_index {
> struct ewah_bitmap *blobs;
> struct ewah_bitmap *tags;
>
> + /*
> + * Type index arrays when this bitmap is associated with an
> + * incremental multi-pack index chain.
> + *
> + * If n is the number of unique layers in the MIDX chain, then
> + * commits_all[n-1] is this structs 'commits' field,
> + * commits_all[n-2] is the commits field of this bitmap's
> + * 'base', and so on.
> + *
> + * When either associated either with a non-incremental MIDX, or
> + * a single packfile, these arrays each contain a single
> + * element.
> + */
> + struct ewah_bitmap **commits_all;
> + struct ewah_bitmap **trees_all;
> + struct ewah_bitmap **blobs_all;
> + struct ewah_bitmap **tags_all;
OK, so these are valid only for the top-level of the chain? I guess
there would not be much point in having the lower levels know about
their incremental versions.
> -static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git)
> +static void load_all_type_bitmaps(struct bitmap_index *bitmap_git)
> +{
> + struct bitmap_index *curr = bitmap_git;
> + size_t i = bitmap_git->base_nr;
> +
> + ALLOC_ARRAY(bitmap_git->commits_all, bitmap_git->base_nr + 1);
> + ALLOC_ARRAY(bitmap_git->trees_all, bitmap_git->base_nr + 1);
> + ALLOC_ARRAY(bitmap_git->blobs_all, bitmap_git->base_nr + 1);
> + ALLOC_ARRAY(bitmap_git->tags_all, bitmap_git->base_nr + 1);
> +
> + while (curr) {
> + bitmap_git->commits_all[i] = curr->commits;
> + bitmap_git->trees_all[i] = curr->trees;
> + bitmap_git->blobs_all[i] = curr->blobs;
> + bitmap_git->tags_all[i] = curr->tags;
> +
> + curr = curr->base;
> + if (curr && !i)
> + BUG("unexpected number of bitmap layers, expected %"PRIu32,
> + bitmap_git->base_nr + 1);
> + i -= 1;
> + }
> +}
It looks like we always allocate these. For the non-incremental case, I
think you could just do:
bitmap_git->commits_all = &bitmap_git->commits;
and so forth. But I doubt that micro-optimization really matters, and it
introduces complications when you have to decide whether to free them or
not.
(And if you really cared about micro-optimizing, probably trying to
prevent the extra pointer-chase in the first place would be a more
productive path).
-Peff
