From 3be6248eb35ded6558444ed4af99316d350b7f03 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 17:18:41 +0200 Subject: [PATCH 1/8] revision: move bloom keyvec precondition into function There are currently two callsites calling check_maybe_different_in_bloom_filter(). They both check if revs->bloom_keyvecs_nr is not zero before they call that function. Move bloom_keyvecs_nr precondition into check_maybe_different_in_bloom_filter() to simplify the code. Note that this changes `bloom_ret` to become -1 when there are no Bloom key vectors, which results in `count_bloom_filter_false_positive` not being incremented. This is unobservable, as the Bloom statistics are only reported when key vectors were set up. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- revision.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/revision.c b/revision.c index ccbe2e03d1..087d64d572 100644 --- a/revision.c +++ b/revision.c @@ -752,6 +752,9 @@ static int check_maybe_different_in_bloom_filter(struct rev_info *revs, struct bloom_filter *filter; int result = 0; + if (!revs->bloom_keyvecs_nr) + return -1; + if (commit_graph_generation(commit) == GENERATION_NUMBER_INFINITY) return -1; @@ -806,7 +809,7 @@ static int rev_compare_tree(struct rev_info *revs, return REV_TREE_SAME; } - if (revs->bloom_keyvecs_nr && !nth_parent) { + if (!nth_parent) { bloom_ret = check_maybe_different_in_bloom_filter(revs, commit); if (bloom_ret == 0) @@ -833,7 +836,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit, if (!t1) return 0; - if (!nth_parent && revs->bloom_keyvecs_nr) { + if (!nth_parent) { bloom_ret = check_maybe_different_in_bloom_filter(revs, commit); if (!bloom_ret) return 1; From 22f3035df65dc329d653a3fad8ec0cb5e61b68d5 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 17:18:42 +0200 Subject: [PATCH 2/8] revision: expose check for paths maybe changed in Bloom filter check_maybe_different_in_bloom_filter() looks up a commit's changed-path Bloom filter and consults it to see whether the commit might have modified any of the paths in the pathspec that `revs` was set up with. In a follow-up commit we want to reuse this logic from another builtin. That caller, however, has already looked up the commit's Bloom filter for its own purposes, so having the function look it up again would mean a redundant lookup. Extract the filter-consulting part into a new public function, revs_maybe_changed_in_bloom(). This function takes an already looked-up `struct bloom_filter` instead of a commit. The existing check_maybe_different_in_bloom_filter() becomes a thin wrapper that looks up the filter and delegates. Expose the new function via revision.h so other builtins can reuse the exact same filtering that `git log ` performs. The existing function check_maybe_different_in_bloom_filter() returns a tristate value. This returns either: * `-1` : No Bloom filter was used. * `0` : The commit definitely did not change any of the paths. * `1` : The commit maybe changed one of the paths. These return values are used to keep count of false-positives. But because the new function revs_maybe_changed_in_bloom() is not involved in counting statistics, it returns a boolean value telling whether the commit definitely did not change any of the paths, or maybe changed some of them. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- revision.c | 30 ++++++++++++++++++++---------- revision.h | 12 ++++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/revision.c b/revision.c index 087d64d572..3628ef963d 100644 --- a/revision.c +++ b/revision.c @@ -750,7 +750,6 @@ static int check_maybe_different_in_bloom_filter(struct rev_info *revs, struct commit *commit) { struct bloom_filter *filter; - int result = 0; if (!revs->bloom_keyvecs_nr) return -1; @@ -765,18 +764,29 @@ static int check_maybe_different_in_bloom_filter(struct rev_info *revs, return -1; } - for (size_t nr = 0; !result && nr < revs->bloom_keyvecs_nr; nr++) { - result = bloom_filter_contains_vec(filter, - revs->bloom_keyvecs[nr], - revs->bloom_filter_settings); + if (revs_maybe_changed_in_bloom(revs, filter)) { + count_bloom_filter_maybe++; + return 1; } - if (result) - count_bloom_filter_maybe++; - else - count_bloom_filter_definitely_not++; + count_bloom_filter_definitely_not++; - return result; + return 0; +} + +bool revs_maybe_changed_in_bloom(struct rev_info *revs, + struct bloom_filter *filter) +{ + if (!revs->bloom_keyvecs_nr || !filter) + return true; + + for (size_t nr = 0; nr < revs->bloom_keyvecs_nr; nr++) + if (bloom_filter_contains_vec(filter, + revs->bloom_keyvecs[nr], + revs->bloom_filter_settings)) + return true; + + return false; } static int rev_compare_tree(struct rev_info *revs, diff --git a/revision.h b/revision.h index 569b3fa1cb..14be39e20c 100644 --- a/revision.h +++ b/revision.h @@ -68,6 +68,7 @@ struct string_list; struct saved_parents; struct follow_pathspec_slab; struct bloom_keyvec; +struct bloom_filter; struct bloom_filter_settings; struct option; struct parse_opt_ctx_t; @@ -493,6 +494,17 @@ void reset_revision_walk(void); */ int prepare_revision_walk(struct rev_info *revs); +/** + * Consult a changed-path Bloom filter to determine if the commit to which the + * filter belongs might have changed any of the paths in the `revs`. + * prepare_revision_walk() needs to be called in advance to ensure + * pathspec key vectors are set up. + * + * Returns false iff the commit definitely did not change any of the paths. + */ +bool revs_maybe_changed_in_bloom(struct rev_info *revs, + struct bloom_filter *filter); + /* Drain the commits linked list into the priority queue. */ void rev_info_commit_list_to_queue(struct rev_info *revs); /** From 316a3e4c3b76a19f3f757c42a6848b9af6c1bea7 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 17:18:43 +0200 Subject: [PATCH 3/8] bloom: add helper to check if any key in a vector is present The changed-path Bloom filter of a commit stores a key for every changed path together with each of its leading directories. To query if a path was changed, bloom_keyvec_new() fills a key vector the same way: a key for the given path and one for each of its leading directories. For example, for "a/b/c" the vector holds keys for "a/b/c", "a/b" and "a". A Bloom filter can only ever prove absence. When a key is not in the filter, the path it was made for definitely did not change. When it is in the filter, the path may have changed, as the key can be a false positive. bloom_filter_contains_vec() looks up all keys of a vector and reports whether all of them are present. That answers: Is this path maybe changed by this commit? A caller that also cares about the directories containing the path asks a different question: Is this path, or any directory leading up to it, maybe changed by this commit? Consider the Bloom filter of a commit that changed "a/b/d". It holds keys for "a/b/d", "a/b" and "a", so looking up the vector of "a/b/c" with bloom_filter_contains_vec() reports that nothing changed, even though "a/b" and "a" did. Add bloom_filter_contains_any_vec(), which reports whether any key in the vector is present. It returns 0 only when none of the keys are in the filter, which means the path and all directories leading up to it definitely did not change. There are no callers yet, one is added in a subsequent commit. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- bloom.c | 12 ++++++++++++ bloom.h | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/bloom.c b/bloom.c index c98d1672ad..192692e548 100644 --- a/bloom.c +++ b/bloom.c @@ -607,6 +607,18 @@ int bloom_filter_contains_vec(const struct bloom_filter *filter, return ret; } +int bloom_filter_contains_any_vec(const struct bloom_filter *filter, + const struct bloom_keyvec *vec, + const struct bloom_filter_settings *settings) +{ + int ret = 0; + + for (size_t nr = 0; !ret && nr < vec->count; nr++) + ret = bloom_filter_contains(filter, &vec->key[nr], settings); + + return ret; +} + uint32_t test_bloom_murmur3_seeded(uint32_t seed, const char *data, size_t len, int version) { diff --git a/bloom.h b/bloom.h index 92ab2100d3..f508db23ad 100644 --- a/bloom.h +++ b/bloom.h @@ -164,6 +164,17 @@ int bloom_filter_contains_vec(const struct bloom_filter *filter, const struct bloom_keyvec *v, const struct bloom_filter_settings *settings); +/* + * bloom_filter_contains_any_vec - Check if any key in a key vector is in the + * Bloom filter. + * + * Returns 1 if **any** key in the vector is present in the filter, 0 if none + * of them are. + */ +int bloom_filter_contains_any_vec(const struct bloom_filter *filter, + const struct bloom_keyvec *v, + const struct bloom_filter_settings *settings); + uint32_t test_bloom_murmur3_seeded(uint32_t seed, const char *data, size_t len, int version); From 923943273486e9eb530fa46a2905863c3bf5fd24 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 17:18:44 +0200 Subject: [PATCH 4/8] revision: add Bloom check that includes parent directories revs_maybe_changed_in_bloom() reports whether a commit may have changed any of the paths in the pathspec. It uses bloom_filter_contains_vec(), which requires all keys of a path's key vector to be present, so it only answers for the paths themselves. A caller may track more than those paths. git-last-modified(1) with --show-trees reports the last modifying commit for the tree entries containing the paths as well, up to the root. For a pathspec "a/b/c/" that means it reports "a" and "a/b" next to "a/b/c" and its entries, and those can each resolve to a different commit. A commit that only changed "a/top" is the answer for "a", even though it touched nothing under "a/b". Such a caller needs to know whether the path, or any of the directories leading up to it, may have changed. Add revs_maybe_changed_in_bloom_with_parents(), which asks that question by using bloom_filter_contains_any_vec() instead. A key vector holds a key for the path and one for each of its leading directories, so looking up any of them answers it. There are no callers yet, one is added in a subsequent commit. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- revision.c | 15 +++++++++++++++ revision.h | 8 ++++++++ 2 files changed, 23 insertions(+) diff --git a/revision.c b/revision.c index 3628ef963d..b56608d144 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) +{ + 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 14be39e20c..58b60f9b3e 100644 --- a/revision.h +++ b/revision.h @@ -505,6 +505,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. + */ +bool revs_maybe_changed_in_bloom_with_parents(struct rev_info *revs, + struct bloom_filter *filter); + /* Drain the commits linked list into the priority queue. */ void rev_info_commit_list_to_queue(struct rev_info *revs); /** From 287e8523915aadda12a25757732ef2a21439d897 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 17:18:45 +0200 Subject: [PATCH 5/8] last-modified: check pathspec against Bloom filter first 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. 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. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- builtin/last-modified.c | 12 ++++++++++++ t/t8020-last-modified.sh | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/builtin/last-modified.c b/builtin/last-modified.c index 5478182f2e..5678731a04 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c @@ -272,6 +272,18 @@ static bool maybe_changed_path(struct last_modified *lm, if (!filter) return true; + /* + * With --show-trees we also track the tree entries containing the + * paths, so a change to any of those parent directories matters too. + */ + if (lm->show_trees) { + if (!revs_maybe_changed_in_bloom_with_parents(&lm->rev, filter)) + return false; + } else { + if (!revs_maybe_changed_in_bloom(&lm->rev, filter)) + return false; + } + hashmap_for_each_entry(&lm->paths, &iter, ent, hashent) { if (active && !bitmap_get(active, ent->diff_idx)) continue; diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh index 9dba4b9d90..df73c7d0d0 100755 --- a/t/t8020-last-modified.sh +++ b/t/t8020-last-modified.sh @@ -269,6 +269,27 @@ test_expect_success 'last-modified merge undoes changes' ' EOF ' +test_expect_success 'last-modified with Bloom filters and --show-trees' ' + test_when_finished rm -rf bloom && + git init bloom && + ( + cd bloom && + mkdir d && + test_commit base-a d/a && + test_commit base-b d/b && + test_commit touch-a d/a && + test_commit touch-b d/b && + + git commit-graph write --reachable --changed-paths && + git -c core.commitGraph=false last-modified -t HEAD -- d/a \ + >expect && + git -c core.commitGraph=true last-modified -t HEAD -- d/a \ + >actual && + + test_cmp expect actual + ) +' + test_expect_success 'cannot run last-modified on two commits' ' test_must_fail git last-modified HEAD HEAD~1 2>err && test_grep "last-modified can only operate on one commit at a time" err From 89481567756a0c169309c6efbb8ab45c80dad19f Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 17:18:46 +0200 Subject: [PATCH 6/8] last-modified: keep per-path Bloom filters for wildcard pathspecs The last-modified builtin expands the pathspec to a set of literal paths and builds a Bloom key for each. During the walk it looks those keys up in the commit's filter to decide whether the commit is worth diffing. These lookups need `bloom_filter_settings` for the key hashing. prepare_revision_walk() runs prepare_to_use_bloom_filter() to build the pathspec key vectors. For a pathspec that cannot be turned into a Bloom key, such as a top-level wildcard like "*.c", that function gives up and clears `bloom_filter_settings`. Restore `bloom_filter_settings` after prepare_revision_walk() so the per-path check keeps working for wildcard pathspecs. This change isn't having any effect on the output, but only has an impact on performance. Add a "bloom_queries" trace2 counter that records how often the per-path Bloom check runs, and a test that asserts the count increments as appropriate for a top-level wildcard pathspec. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- builtin/last-modified.c | 16 ++++++++++++++++ t/t8020-last-modified.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/builtin/last-modified.c b/builtin/last-modified.c index 5678731a04..2c4343603d 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c @@ -18,6 +18,7 @@ #include "quote.h" #include "repository.h" #include "revision.h" +#include "trace2.h" /* Remember to update object flag allocation in object.h */ #define PARENT1 (1u<<16) /* used instead of SEEN */ @@ -63,6 +64,8 @@ struct last_modified { /* 'scratch' to avoid allocating a bitmap every process_parent() */ struct bitmap *scratch; + + unsigned int count_bloom_filter_queries; }; static struct bitmap *active_paths_for(struct last_modified *lm, struct commit *c) @@ -272,6 +275,8 @@ static bool maybe_changed_path(struct last_modified *lm, if (!filter) return true; + lm->count_bloom_filter_queries++; + /* * With --show-trees we also track the tree entries containing the * paths, so a change to any of those parent directories matters too. @@ -369,6 +374,14 @@ static int last_modified_run(struct last_modified *lm) prepare_revision_walk(&lm->rev); + /* + * 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); + max_count = lm->rev.max_count; init_active_paths_for_commit(&lm->active_paths); @@ -476,6 +489,9 @@ cleanup: if (hashmap_get_size(&lm->paths)) BUG("paths remaining beyond boundary in last-modified"); + trace2_data_intmax("last-modified", lm->rev.repo, "bloom_queries", + lm->count_bloom_filter_queries); + clear_prio_queue(¬_queue); clear_prio_queue(&queue); clear_active_paths_for_commit(&lm->active_paths); diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh index df73c7d0d0..75b18ee83b 100755 --- a/t/t8020-last-modified.sh +++ b/t/t8020-last-modified.sh @@ -290,6 +290,32 @@ test_expect_success 'last-modified with Bloom filters and --show-trees' ' ) ' +test_expect_success 'last-modified with Bloom filters and top-level wildcard' ' + test_when_finished rm -rf wildcard && + git init wildcard && + ( + cd wildcard && + test_commit base-c a.c && + test_commit base-h a.h && + test_commit touch-c a.c && + mkdir d && + test_commit sub-c d/b.c && + + git commit-graph write --reachable --changed-paths && + GIT_TRACE2_PERF="$(pwd)/off.perf" \ + git -c core.commitGraph=false last-modified -r HEAD \ + -- "*.c" >expect && + test_grep "data .* bloom_queries:0$" off.perf && + + GIT_TRACE2_PERF="$(pwd)/on.perf" \ + git -c core.commitGraph=true last-modified -r HEAD \ + -- "*.c" >actual && + test_grep "data .* bloom_queries:2$" on.perf && + + test_cmp expect actual + ) +' + test_expect_success 'cannot run last-modified on two commits' ' test_must_fail git last-modified HEAD HEAD~1 2>err && test_grep "last-modified can only operate on one commit at a time" err From 632e11971bba976f7fca647fc060ef5afbbd2d12 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 31 Aug 2026 21:22:55 -0700 Subject: [PATCH 7/8] fixup! last-modified: check pathspec against Bloom filter first --- t/t8020-last-modified.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh index 75b18ee83b..a1ff351822 100755 --- a/t/t8020-last-modified.sh +++ b/t/t8020-last-modified.sh @@ -281,8 +281,10 @@ test_expect_success 'last-modified with Bloom filters and --show-trees' ' test_commit touch-b d/b && git commit-graph write --reachable --changed-paths && + GIT_TEST_COMMIT_GRAPH=0 \ git -c core.commitGraph=false last-modified -t HEAD -- d/a \ >expect && + GIT_TEST_COMMIT_GRAPH=1 \ git -c core.commitGraph=true last-modified -t HEAD -- d/a \ >actual && From 92cf54eec6d173969af445c18bc47aab80bbea79 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 31 Aug 2026 21:23:09 -0700 Subject: [PATCH 8/8] fixup! last-modified: keep per-path Bloom filters for wildcard pathspecs --- t/t8020-last-modified.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh index a1ff351822..fb46f38772 100755 --- a/t/t8020-last-modified.sh +++ b/t/t8020-last-modified.sh @@ -304,11 +304,13 @@ test_expect_success 'last-modified with Bloom filters and top-level wildcard' ' test_commit sub-c d/b.c && git commit-graph write --reachable --changed-paths && + GIT_TEST_COMMIT_GRAPH=0 \ GIT_TRACE2_PERF="$(pwd)/off.perf" \ git -c core.commitGraph=false last-modified -r HEAD \ -- "*.c" >expect && test_grep "data .* bloom_queries:0$" off.perf && + GIT_TEST_COMMIT_GRAPH=1 \ GIT_TRACE2_PERF="$(pwd)/on.perf" \ git -c core.commitGraph=true last-modified -r HEAD \ -- "*.c" >actual &&