git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 1/2] commit-graph: bump DIE_ON_LOAD check to actual load-time

From
Jeff King <peff@peff.net>
Date
Sep 12, 2019, 14:44 UTC
Message-ID
<20190912144434.GA25101@sigill.intra.peff.net>
In-Reply-To
<20190912144122.GA27815@sigill.intra.peff.net>

Commit 43d3561805 (commit-graph write: don't die if the existing graph is corrupt, 2019-03-25) added an environment variable we use only in the test suite, $GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD. But it put the check for this variable at the very top of prepare_commit_graph(), which is called every time we want to use the commit graph. Most importantly, it comes _before_ we check the fast-path "did we already try to load?", meaning we end up calling getenv() for every single use of the commit graph, rather than just when we load.

getenv() is allowed to have unexpected side effects, but that shouldn't be a problem here; we're lazy-loading the graph so it's clear that at least _one_ invocation of this function is going to call it.

But it is inefficient. getenv() typically has to do a linear search through the environment space.

We could memoize the call, but it's simpler still to just bump the check down to the actual loading step. That's fine for our sole user in t5318, and produces this minor real-world speedup:

  [before]
  Benchmark #1: git -C linux rev-list HEAD >/dev/null
    Time (mean ± σ):      1.460 s ±  0.017 s    [User: 1.174 s, System: 0.285 s]
    Range (min … max):    1.440 s …  1.491 s    10 runs
  [after]
  Benchmark #1: git -C linux rev-list HEAD >/dev/null
    Time (mean ± σ):      1.391 s ±  0.005 s    [User: 1.118 s, System: 0.273 s]
    Range (min … max):    1.385 s …  1.399 s    10 runs

Of course that actual speedup depends on how big your environment is. We can game it like this:

  for i in $(seq 10000); do
    export dummy$i=$i
  done
in which case I get:
  [before]
  Benchmark #1: git -C linux rev-list HEAD >/dev/null
    Time (mean ± σ):      6.257 s ±  0.061 s    [User: 6.005 s, System: 0.250 s]
    Range (min … max):    6.174 s …  6.337 s    10 runs
  [after]
  Benchmark #1: git -C linux rev-list HEAD >/dev/null
  Time (mean ± σ):      1.403 s ±  0.005 s    [User: 1.146 s, System: 0.256 s]
  Range (min … max):    1.396 s …  1.412 s    10 runs

So this is really more about avoiding the pathological case than providing a big real-world speedup.

Signed-off-by: Jeff King <peff@peff.net>
---
 commit-graph.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 9b02d2c426..baeaf0d1bf 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -468,14 +468,14 @@ static int prepare_commit_graph(struct repository *r)
 {
 	struct object_directory *odb;
 
-	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
-		die("dying as requested by the '%s' variable on commit-graph load!",
-		    GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
-
 	if (r->objects->commit_graph_attempted)
 		return !!r->objects->commit_graph;
 	r->objects->commit_graph_attempted = 1;
 
+	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
+		die("dying as requested by the '%s' variable on commit-graph load!",
+		    GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
+
 	prepare_repo_settings(r);
 
 	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
-- 
2.23.0.667.gcccf1fbb03
Previous: Jeff KingNext: Derrick Stolee
Message 21 of 25 in “upload-pack: disable commit graph more gently for shallow traversal”
  1. upload-pack: disable commit graph more gently for shallow traversalJeff King, Sep 12, 2019
  2. Jeff KingSep 12, 2019
  3. list-objects: don't queue root trees unless revs->tree_objects is setJeff King, Sep 12, 2019
  4. Jeff KingSep 12, 2019
  5. Derrick StoleeSep 12, 2019
  6. Junio C HamanoSep 12, 2019
  7. Jeff KingSep 12, 2019
  8. Junio C HamanoSep 13, 2019
  9. Taylor BlauSep 12, 2019
  10. Jeff KingSep 12, 2019
  11. Taylor BlauSep 12, 2019
  12. SZEDER GáborSep 12, 2019
  13. Jeff KingSep 12, 2019
  14. Derrick StoleeSep 12, 2019
  15. Jeff KingSep 12, 2019
  16. Derrick StoleeSep 12, 2019
  17. Jeff KingSep 12, 2019
  18. Derrick StoleeSep 12, 2019
  19. upload-pack commit graph segfault fixJeff King, Sep 12, 2019
  20. Jeff KingSep 12, 2019
  21. 1/2 commit-graph: bump DIE_ON_LOAD check to actual load-timeJeff King, Sep 12, 2019
  22. Derrick StoleeSep 12, 2019
  23. 2/2 upload-pack: disable commit graph more gently for shallow traversalJeff King, Sep 12, 2019
  24. Derrick StoleeSep 13, 2019
  25. Taylor BlauSep 12, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.