Re: [PATCH v4 5/6] last-modified: check pathspec against Bloom filter first

On Tue, Sep 01, 2026 at 11:10:25AM +0200, Toon Claes wrote:

When git-last-modified(1) starts, it builds a list of all the paths
matching the pathspec it needs to find the last modifying commit for.
For example, every file and subdirectory listed by:

    $ git last-modified -t --max-depth=0 -- src/

As it resolves a commit for each path during the revision walk, it drops
that path from the list.

To avoid diffing trees for every commit, Bloom filters are used when
available. For each remaining path, the commit's Bloom filter is checked
to see whether the commit changed that path. The Bloom filter says
either "no" or "maybe", and only in the latter case is the diff
calculated.

git-log(1) does this differently. It does not expand the pathspec but
checks the Bloom filter against the pathspec itself. This way, commits
not touching any path matching the pathspec can be discarded as a whole.

Apply this same check to git-last-modified(1). In a previous commit the
function revs_maybe_changed_in_bloom(), used by git-log(1), was made
public. Use this as a pre-filter in git-last-modified(1). After this
pre-filter, paths are still checked one-by-one to only find those which
don't have a "last commit" yet.

So in theory, we _might_ now do some of the checks multiple times. But
the expectation is that the number of pathspecs is typically much lower
than the number of expanded paths to check against, so in most cases it
should be faster to do this pre-filtering?

It'll probably be possible to craft edge cases where the new logic is
slower because we now do more work in the matching case. But overall I
think this is a sensible tradeoff. After all, we use the same tradeoff
in git-log(1).

With `--show-trees` the list holds more than the paths matching the
pathspec. It also holds each parent tree entry, up to the root. Each of
those can resolve to a different commit. Thus for the pathspec "a/b/c",
the list will also hold "a" and "a/b".

When a commit touches "a/other", that commit could be the last commit
for "a", but revs_maybe_changed_in_bloom() would discard it, because it
doesn't match the full pathspec.

Instead, when `--show-trees` is given, use
revs_maybe_changed_in_bloom_with_parents(), which indicates the commit
maybe changed any of the paths leading up to the path in the pathspec.

You explain what we do and why it's safe, which is good. But what's
missing is the "why". As far as I understand the reason is performance,
but if so I'd have expected a benchmark demonstrating the benefit.

Patrick