Re: [PATCH 4/4] last-modified: keep per-path Bloom filters for wildcard pathspecs

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

Jeff King <peff@peff.net> writes:

> On Fri, Jul 17, 2026 at 09:16:34PM +0200, Toon Claes wrote:
>
>> > +	/*
>> > +	 * prepare_revision_walk() clears bloom_filter_settings for pathspecs
>> > +	 * without a Bloom key. Restore it so the per-path check keeps working.
>> > +	 */
>> > +	if (!lm->rev.bloom_filter_settings)
>> > +		lm->rev.bloom_filter_settings =
>> > +			get_bloom_filter_settings(lm->rev.repo);
>> > +
>> 
>> @Peff, as far I could tell:
>> 
>> * This change was not needed to be able to use the Bloom filters with
>>   the pathspec.
>
> Ah, right. In my earlier attempt I came at it from the bottom up: I
> found the bloom_keyvec, saw how it was populated, and then worked my way
> back to prepare_to_use_bloom_filter() without going further.
>
> But it is much nicer if we can rely on prepare_revision_walk() here, as
> we don't need to make an additional function public.
> ...
> It's mostly academic, as both of the pointers (if not NULL) would always
> point to the same setting that ultimately come from the repository
> object. But it feels cleaner for them to keep their own pointers,
> because that pointer may also signal "do we have usable bloom filters".
> We are a little lucky in dodging a bug here: last-modified uses the
> pointer for that purpose, but if revision.c did so also, they'd
> conflict.
>
>   Side note: this is really a repository property, so it would be nice
>   if we could just do:
>
>     repo_bloom_filter_contains(filter, &ent->key);
>
>   without managing the settings pointer ourselves at all. But the cost
>   to fetch it from the graph linked list is not totally trivial, so we'd
>   probably end up having to cache it somewhere. I don't know if that's
>   worth it (plus last-modified would still have to keep a boolean
>   somewhere to decide whether it is using bloom filters or not).

So what happened to this discussion?  Are we happy with the set of
patches in v1 after all, or are we still thinking it over?

Thanks.