Re: [PATCH 9/9] builtin/cat-file: use bitmaps to efficiently filter by object type
From: Taylor Blau <hidden>
Date: 2025-02-27 23:48:25
On Fri, Feb 21, 2025 at 08:47:34AM +0100, Patrick Steinhardt wrote:
quoted hunk ↗ jump to hunk
@@ -813,9 +827,40 @@ static void batch_each_object(for_each_object_fn callback, .callback = callback, .payload = _payload, }; + struct bitmap_index *bitmap = prepare_bitmap_git(the_repository); + for_each_loose_object(batch_one_object_loose, &payload, 0); - for_each_packed_object(the_repository, batch_one_object_packed, - &payload, flags); + + if (bitmap && + (opt->objects_filter.choice == LOFC_OBJECT_TYPE || + opt->objects_filter.choice == LOFC_BLOB_NONE)) {
Makes sense. I think there is one more case here that we could handle,
which is
opt->objects_filter.choice == LOFC_TREE_DEPTH && opt->objects_filter.depth == 0
where we'd just want to show commits.
I am scratching my head on if there is a convenient way to unify this
logic with pack-bitmap.c::filter_bitmap(). I think there is, but there
are a couple of wrinkles:
- filter_bitmap() is really designed to work with a whole 'struct
bitmap', and doesn't know how to deal with an ewah_iterator.
- traverse_bitmap_commit_list() is designed to provide a way for
callers to iterate over the set of objects reachable for some
rev-list query.
There we *do* have good facilities for iterating over an
ewah_iterator, which is what you'd want. But that function really
wants to have performed a bitmap walk first (see the
"assert(bitmap_git->result)" call at the beginning of that
function).
The new pieces of batch_each_object() introduced in this patch are
tantalizingly close to much of the existing logic in pack-bitmap.c. I
think there is a way to unify them by introducing a way to traverse over
the bitmap as a whole as if bitmap_git->result were the all-1s bitmap.
Thanks,
Taylor