On Fri, Mar 14, 2025 at 04:18:24PM -0400, Taylor Blau wrote:
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 6406953d32..c26d85b5db 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -170,6 +170,15 @@ static struct ewah_bitmap *read_bitmap_1(struct bitmap_index *index)
> return read_bitmap(index->map, index->map_size, &index->map_pos);
> }
>
> +static uint32_t bitmap_non_extended_bits(struct bitmap_index *index)
> +{
> + if (index->midx) {
> + struct multi_pack_index *m = index->midx;
> + return m->num_objects + m->num_objects_in_base;
> + }
> + return index->pack->num_objects;
> +}
I understand why we need to account for the objects in the base to
offset our total size.
Similar to Patrick's comments on v3, I wondered about why we couldn't
just modify bitmap_num_objects() here, and why some callers would be
left with the other.
I guess sometimes we still need to consider a single layer. We can't
quite just access m->num_objects there, because we still need the midx
vs pack abstraction layer. I just thought there'd be more discussion
here, but it looks the same as v3.
I wonder if it is worth renaming bitmap_num_objects() to indicate that
it is a single layer (and make sure other callers are examined). I
dunno.
I also suspect from previous forays into bitmap indexing that it will be
easy to mix up positions in various units (local to the layer vs in the
global pseudo-pack ordering, for example). In theory we could use types
to help us with this, but they're kind of weak in C (unless we wrap all
of the ints in structs). Maybe not worth it.
-Peff
