Re: [PATCH 6/9] pack-bitmap: expose function to iterate over bitmapped objects

[Taylor Blau <me@ttaylorr.com>]

On Thu, Feb 27, 2025 at 03:32:32PM -0800, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > It looks like the aim here is to introduce a function which executes a
> > callback for each object of some type in a bitmap. That's a thin wrapper
> > over the ewah_iterator, but it's not clear why we need a wrapper around
> > that function since it is internal to pack-bitmap.c. Likewise, this is a
> > performance critical area, so I am not sure I'm in favor of adding a
> > function pointer to a hot path which executes once per object for some
> > object type.
>
> It internally introduced ewah_for_type(), giving the "struct
> bitmap_index" object an abstraction that callers can ask for the
> bitmap for any type the caller wants.  Before the <type>_all bitmaps
> were introduced, there were one ewah-bitmap per type, so it made
> sense for a caller to ask "Now, for this bitmap_index, give me the
> ewah-bitmap for commits", but with "commits_all" added to the
> bitmap_index object, it is no longer clear to me what the answer to
> that question should be.

I think these are orthogonal. (FWIW, I think the correct answer would be
"commits_all" in that world, but that is definitely out of scope for
Patrick's immediate concern). In any event, I see that later on in the
series it is important for callers to enumerate bitmapped objects of a
certain type. So having a callback to do that makes sense.

Thanks,
Taylor