Derrick Stolee <stolee@xxxxxxxxx> 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.
