On Tue, Feb 25, 2025 at 07:59:14AM +0100, Patrick Steinhardt wrote:

On Mon, Feb 24, 2025 at 10:05:27AM -0800, Junio C Hamano wrote:

quoted

Patrick Steinhardt [off-list ref] writes:

quoted

Expose a function that allows the caller to iterate over all bitmapped
objects of a specific type. This mechanism allows us to use the object
type-specific bitmaps to enumerate all objects of that type without
having to scan through a complete packfile.

This functionality will be used in a subsequent commit.

Signed-off-by: Patrick Steinhardt <redacted>
---
 builtin/pack-objects.c |  3 ++-
 builtin/rev-list.c     |  3 ++-
 pack-bitmap.c          | 65 +++++++++++++++++++++++++++++++-------------------
 pack-bitmap.h          | 12 +++++++++-
 reachable.c            |  3 ++-
 5 files changed, 57 insertions(+), 29 deletions(-)

After 2189649b (pack-bitmap.c: keep track of each layer's type
bitmaps, 2024-11-19) added <type>_all bitmaps to the bitmap_index
struct, this step would need some adjustment, I am afraid.

Hm, does it? I understand that this commit only makes the bitmaps
accessible individually per bitmapped packfile, but the bitmap indices
part of `struct bitmap_index` would continue to be the union of all of
those bitmaps. Oh, but that changes in the subsequent commits indeed,
where we start to use an `ewah_or_iterator`.

That's right; the ewah_or_iterator is the mechanism by which we can
combine multiple "layers" of the bitmaps into a single iterator.

(As an aside, that was not the first approach I pursued. Initially the
caller was supposed to chase the 'next' pointer of each bitmap and
enumerate through whatever type iterator they're interested in at each
layer. But that was too error-prone, since you have to remember and
update the offset into the pseudo-pack order across multiple layers.)

I see that Taylor's series has been sitting in an unreviewed state for a
couple months already. I can review it with the hope of moving it
forward and can then pull it in as a dependency of this series. But I'll
wait for him to chime in first to see whether anything changed about its
current state.

It would be great to get some review from you on that series. I know
that it has been on Peff's (CC'd) radar for a while, but that he has
likewise had a few off-list things to deal with lately as well.

I am still not sold on introducing a callback here, though, and would
much rather see callers interact with the iterator directly.

Thanks,
Taylor