Skip to content

Commit fbab552

Browse files
peffgitster
authored andcommitted
commit-graph: bump DIE_ON_LOAD check to actual load-time
Commit 43d3561 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent f1d4a28 commit fbab552

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

commit-graph.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -468,14 +468,14 @@ static int prepare_commit_graph(struct repository *r)
468468
{
469469
struct object_directory *odb;
470470

471-
if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
472-
die("dying as requested by the '%s' variable on commit-graph load!",
473-
GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
474-
475471
if (r->objects->commit_graph_attempted)
476472
return !!r->objects->commit_graph;
477473
r->objects->commit_graph_attempted = 1;
478474

475+
if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
476+
die("dying as requested by the '%s' variable on commit-graph load!",
477+
GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
478+
479479
prepare_repo_settings(r);
480480

481481
if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&

0 commit comments

Comments
 (0)