Re: [PATCH 9/9] builtin/cat-file: use bitmaps to efficiently filter by object type

[Taylor Blau <me@ttaylorr.com>]

On Fri, Feb 21, 2025 at 08:47:34AM +0100, Patrick Steinhardt wrote:
> @@ -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