Re: [PATCH v4 4/4] bloom: optimize multiple pathspec items in revision traversal
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 7, 2025, 15:14 UTC
- Message-ID
- <xmqqikk458y7.fsf@gitster.g>
- In-Reply-To
- <ea144a72-0975-4ac9-b2e4-ae0f7fcb6837@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
Show 22 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).
Excellent suggestion. Very much appreciated.
Show 7 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 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.