Re: [PATCH v4 4/4] bloom: optimize multiple pathspec items in revision traversal
- From
Lidong Yan <502024330056@smail.nju.edu.cn>
- Date
- Jul 7, 2025, 14:18 UTC
- Message-ID
- <23232D29-CEFA-409D-90E3-F894EC37C1BE@smail.nju.edu.cn>
- In-Reply-To
- <ea144a72-0975-4ac9-b2e4-ae0f7fcb6837@gmail.com>
Derrick Stolee <stolee@gmail.com> wrote:
Show 23 quoted lines
> > 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).
Got it, I will add statistics in the next version.
>> + int path_component_nr; > > We can move this into the interior of the loop, right?
Yes, move this into loop looks better.
Show 6 quoted lines
> > 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 will try to figure out a way to make diff cleaner.
Thanks, Lidong