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

From: Taylor Blau <me@ttaylorr.com>
Date: Feb 27, 2025, 23:48 UTC
Message-ID: <Z8D5xiAVmWOUJJ1v@nand.local>
In-Reply-To: <20250221-pks-cat-file-object-type-filter-v1-9-0852530888e2@pks.im>

On Fri, Feb 21, 2025 at 08:47:34AM +0100, Patrick Steinhardt wrote:

Show 13 quoted lines

> @@ -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