Re: [PATCH v3 12/13] pack-bitmap.c: use `ewah_or_iterator` for type bitmap iterators

[Patrick Steinhardt <ps@pks.im>]

On Tue, Nov 19, 2024 at 05:07:53PM -0500, Taylor Blau wrote:
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 348488e2d9e..83696d834f6 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -1622,25 +1622,29 @@ static void show_extended_objects(struct bitmap_index *bitmap_git,
>  	}
>  }
>  
> -static void init_type_iterator(struct ewah_iterator *it,
> +static void init_type_iterator(struct ewah_or_iterator *it,
>  			       struct bitmap_index *bitmap_git,
>  			       enum object_type type)
>  {
>  	switch (type) {
>  	case OBJ_COMMIT:
> -		ewah_iterator_init(it, bitmap_git->commits);
> +		ewah_or_iterator_init(it, bitmap_git->commits_all,
> +				      bitmap_git->base_nr);
>  		break;
>  
>  	case OBJ_TREE:
> -		ewah_iterator_init(it, bitmap_git->trees);
> +		ewah_or_iterator_init(it, bitmap_git->trees_all,
> +				      bitmap_git->base_nr);
>  		break;
>  
>  	case OBJ_BLOB:
> -		ewah_iterator_init(it, bitmap_git->blobs);
> +		ewah_or_iterator_init(it, bitmap_git->blobs_all,
> +				      bitmap_git->base_nr);
>  		break;
>  
>  	case OBJ_TAG:
> -		ewah_iterator_init(it, bitmap_git->tags);
> +		ewah_or_iterator_init(it, bitmap_git->tags_all,
> +				      bitmap_git->base_nr);
>  		break;
>  
>  	default:

One thing I wonder here is whether we want to continue using the
non-layered iterator in case we don't have an MIDX. But I guess it makes
it easier to not do it like that because we have less conditionals, and
overall the or'd logic shouldn't perform significantly workse anyway. So
as long as we always initialize `base_nr` and the relevant `*_all`
fields even in the non-MIDX case we're fine.

Patrick