diff --git a/bloom.c b/bloom.c index caf22f9831..b96534e6e3 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); diff --git a/builtin/last-modified.c b/builtin/last-modified.c index 3846244dfc..fe14f4493c 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; @@ -358,6 +370,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); diff --git a/revision.c b/revision.c index 6bd3e3b7ec..54c459eca1 100644 --- a/revision.c +++ b/revision.c @@ -750,7 +750,9 @@ 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; if (commit_graph_generation(commit) == GENERATION_NUMBER_INFINITY) return -1; @@ -762,18 +764,44 @@ 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; +} + +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, @@ -806,7 +834,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 +861,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; diff --git a/revision.h b/revision.h index acf6d06b24..192001ff79 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; @@ -495,6 +496,25 @@ 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); + +/** + * 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); /** 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