From 89481567756a0c169309c6efbb8ab45c80dad19f Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 17:18:46 +0200 Subject: [PATCH] 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