Re: [PATCH 2/2] bloom: enable multiple pathspec bloom keys

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 2, 2025, 15:48 UTC
Message-ID: <xmqq1pqyfvb2.fsf@gitster.g>
In-Reply-To: <D5CB9B7A-C7B2-4F5A-B358-8F46A4E18CDB@gmail.com>

Lidong Yan <yldhome2d2@gmail.com> writes:

Show 10 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> I do not know about icase; though.  Asking about "Dir/Path" and
>> getting "Dir/ or Dir/Path cannot possibly be in the set of paths
>> that were modified" from the changed-path Bloom filter would not
>> help us optimize the tree comparison out, when we do not want to
>> miss modifications for "dir/path".
>
> Make sense, both PATHSPEC_EXCLUDE and PATHSPEC_ICASE shouldn’t be
> optimized by bloom filter.

Before concluding so, we may want to double check how Bloom filters are built on case insensitive systems, though. If we normalize the string by downcasing before murmuring the string, the resulting Bloom filter may have more false positives for those who want to (ab)use it to optimize case sensitive queries (without affecting correctness), but case insensitive queries would be helped. I do not think we support (or want to support) a repository that spans across two filesystems with different case sensitivity, so those who worked on our changed-path Bloom filter subsystem may have already placed such an optimization, based on the case sensitivity recorded in the repository (core.ignorecase).

> I found that my [PATCH v3 2/2] contains two unaligned parameters. Should I reroll
> this patch and introduce the nowildcard_len change in a separate commit?

Updating a patch with a fix to obvious known problems is good.

Extending the scope of the series should be left out for a new separate commit. It may even be a better idea to hold it while the current set of patches are still being polished, and then sent out as a new series after the dust settles (even if you internally developed that part as a direct extension to the current effort).

Thanks.