Re: [PATCH v4 4/4] bloom: optimize multiple pathspec items in revision traversal

[Junio C Hamano <gitster@pobox.com>]

Derrick Stolee <stolee@gmail.com> writes:

> On 7/4/2025 7:14 AM, Lidong Yan wrote:
>> To enable optimize multiple pathspec items in revision traversal,
>> return 0 if all pathspec item is literal in forbid_bloom_filters().
>> Add code to initialize and check each pathspec item's bloom_keyvec.
>> 
>> Add new function release_revisions_bloom_keyvecs() to free all bloom
>> keyvec owned by rev_info.
>> 
>> Add new test cases in t/t4216-log-bloom.sh to ensure
>>   - consistent results between the optimization for multiple pathspec
>>     items using bloom filter and the case without bloom filter
>>     optimization.
>>   - does not use bloom filter if any pathspec item is not literal.
>
> This would be a great time to add some performance statistics when
> using this feature with multiple pathspecs on some standard repos (git
> and the Linux kernel repo are two good examples).
>
> We don't have a great performance script for this, since each test
> repo will have different paths to use for comparisons, but you can
> use 'hyperfine' to assemble your own comparisons before and after
> this change and report them here (and in your cover letter). 

Excellent suggestion.  Very much appreciated.

> The size of this diff is unfortunate. I wonder if there could first
> be an extraction of this logic to operate on a single pathspec and
> bloom_keyvec in a way that would be an obvious code move, then this
> patch could call that method in a loop now that we have an array of
> bloom_keyvecs.
>
> I think it would make a cleaner patch and a cleaner final result.

Hmph.  Didn't think of that extra "preliminary clean-up" step
myself, but I think you have a point.  That would likely make
the resulting series easier to follow.

Thanks.