From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 10 Sep 2026 07:04:10 GMT
Subject: Re: [PATCH v4 4/6] revision: add Bloom check that includes parent directories
Message-ID: <aqJWasW9IKXhjfd7@pks.im>
In-Reply-To: <20260901-toon-speed-up-last-modified-v4-4-a09949800404@iotcl.com>

On Tue, Sep 01, 2026 at 11:10:24AM +0200, Toon Claes wrote:
> diff --git a/revision.c b/revision.c
> index ed46b90b00..a560146b4d 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -789,6 +789,21 @@ bool revs_maybe_changed_in_bloom(struct rev_info *revs,
>  	return false;
>  }
>  
> +bool revs_maybe_changed_in_bloom_with_parents(struct rev_info *revs,
> +					      struct bloom_filter *filter)

In "revision.c", "parents" would immediately read as "commit parent" to
me. Would `revs_maybe_changed_in_bloom_with_leading_dirs()` be a better
name to clarify that this is about directories, only?

> +{
> +	if (!revs->bloom_keyvecs_nr || !filter)
> +		return true;
> +
> +	for (size_t nr = 0; nr < revs->bloom_keyvecs_nr; nr++)
> +		if (bloom_filter_contains_any_vec(filter,
> +						  revs->bloom_keyvecs[nr],
> +						  revs->bloom_filter_settings))
> +			return true;
> +
> +	return false;
> +}
> +
>  static int rev_compare_tree(struct rev_info *revs,
>  			    struct commit *parent, struct commit *commit, int nth_parent)
>  {
> diff --git a/revision.h b/revision.h
> index 67778558e1..192001ff79 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -507,6 +507,14 @@ int prepare_revision_walk(struct rev_info *revs);
>  bool revs_maybe_changed_in_bloom(struct rev_info *revs,
>  				 struct bloom_filter *filter);
>  
> +/**
> + * Same as revs_maybe_changed_in_bloom(), but a change to any of the directories
> + * leading up to a path counts as well. Callers that track the tree entries
> + * containing the paths, and not just the paths themselves, need this.
> + */

This comment also talks about leading directories, not parent
directories.

> +bool revs_maybe_changed_in_bloom_with_parents(struct rev_info *revs,
> +					      struct bloom_filter *filter);
> +

Patrick