Integrate changed-path Bloom filters with 'git blame'

If the changed-path Bloom filters are relatively stable, then I propose trying to build upon them as a way to discover any deficiencies. Also, it's good to use them when we can.

The goal I set out to achieve was to use Bloom filters as much as possible in git blame. I think I have achieved most of that, except I did not integrate it with the -C mode. In that case, the blob-diff computation takes a majority of the computation time, so short-circuiting the tree diff using Bloom filters. Also, it's just really complicated. If someone else thinks there is an easy win there, then please go ahead and give it a try with the extra logic presented here in PATCH 3.

While I was playing around with Bloom filters and git blame, I purposefully got it working with some scenarios but not all. Then I tried to trigger a failing build in the blame tests using GIT_TEST_COMMIT_GRAPH=1 and GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1. But the tests all succeeded!

Examining the data, I see that the commit-graph didn't have the Bloom filter chunks at all. This is because we are not setting the flag to write them in the right spot. The GIT_TEST_COMMIT_GRAPH=1 variable triggers a commit-graph write during git commit, so we need to update the code there instead of just inspecting the variable in git commit-graph write. (This is PATCH 2.)

By updating this variable, I saw some test failures in other tests regarding non-exact pathspecs. I fixed these in PATCH 1 so we keep clean builds.

I based this change on [1] but it would apply cleanly (and logically) on gs/commit-graph-path-filter

Updates in v2:

* Added PATCH 3 to write commit-graph files during 'git merge' if GIT_TEST_COMMIT_GRAPH is enabled.

* Updated PATCH 1 with the simplification recommended by Taylor.

* Fixed the lower-case "bloom" in the commit message.

Thanks, -Stolee

[1]
https://lore.kernel.org/git/[email protected]/

Derrick Stolee (3):
revision: complicated pathspecs disable filters
tests: write commit-graph with Bloom filters
blame: use changed-path Bloom filters

 blame.c          | 139 ++++++++++++++++++++++++++++++++++++++++++++---
 blame.h          |   6 ++
 builtin/blame.c  |  10 ++++
 builtin/commit.c |   4 +-
 builtin/merge.c  |   7 ++-
 commit-graph.c   |  14 +++++
 commit-graph.h   |   9 +++
 revision.c       |  19 ++++++-
 8 files changed, 193 insertions(+), 15 deletions(-)

base-commit: f4df00a0dd448edce0e854a97f63598fefe27d27
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-609%2Fderrickstolee%2Fbloom-blame-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-609/derrickstolee/bloom-blame-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/609

Range-diff vs v2:

 1:  adc03eee4ac = 1:  adc03eee4ac revision: complicated pathspecs disable filters
 2:  7e8f1aed113 < -:  ----------- commit: write commit-graph with Bloom filters
 3:  824f8ad067b ! 2:  4073c8fe42f commit-graph: write commit-graph in more tests

@@ Metadata
Author: Derrick Stolee <[email protected]>

## Commit message ##
- commit-graph: write commit-graph in more tests
+ tests: write commit-graph with Bloom filters

- The GIT_TEST_COMMIT_GRAPH test environment variable triggers
- commit-graph writes during each "git commit" process. To expand
- the number of tests that have commits in the commit-graph file,
- add a helper method that computes the commit-graph and place
+ The GIT_TEST_COMMIT_GRAPH environment variable updates the commit-
+ graph file whenever "git commit" is run, ensuring that we always
+ have an updated commit-graph throughout the test suite. The
+ GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS environment variable was
+ introduced to write the changed-path Bloom filters whenever "git
+ commit-graph write" is run. However, the GIT_TEST_COMMIT_GRAPH
+ trick doesn't launch a separate process and instead writes it
+ directly.
+
+ To expand the number of tests that have commits in the commit-graph
+ file, add a helper method that computes the commit-graph and place
that helper inside "git commit" and "git merge".

+ In the helper method, check GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS
+ to ensure we are writing changed-path Bloom filters whenever
+ possible.
+

## builtin/commit.c ##
@@ commit-graph.c

+void git_test_write_commit_graph_or_die(void)
+{
-+ if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
-+ write_commit_graph_reachable(the_repository->objects->odb, 0, NULL))
++ int flags = 0;
++ if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0))
++ return;
++
++ if (git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0))
++ flags = COMMIT_GRAPH_WRITE_BLOOM_FILTERS;
++
++ if (write_commit_graph_reachable(the_repository->objects->odb,
++ flags, NULL))
+ die("failed to write commit-graph under GIT_TEST_COMMIT_GRAPH");
+}
+
@@ commit-graph.h
#define GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD "GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD" #define GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS "GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS"

++/*
++ * This method is only used to enhance coverage of the commit-graph ++ * feature in the test suite with the GIT_TEST_COMMIT_GRAPH and ++ * GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS environment variables. Do not ++ * call this method oustide of a builtin, and only if you know what ++ * you are doing!
++ */
+void git_test_write_commit_graph_or_die(void);
+
struct commit;
4: 4ae196d6355 = 3: 463d6bf5033 blame: use changed-path Bloom filters

gitgitgadget