Skip to content

Commit 67bb65d

Browse files
peffgitster
authored andcommitted
packfile: actually set approximate_object_count_valid
The approximate_object_count() function tries to compute the count only once per process. But ever since it was introduced in 8e3f52d (find_unique_abbrev: move logic out of get_short_sha1(), 2016-10-03), we failed to actually set the "valid" flag, meaning we'd compute it fresh on every call. This turns out not to be _too_ bad, because we're only iterating through the packed_git list, and not making any system calls. But since it may get called for every abbreviated hash we output, even this can add up if you have many packs. Here are before-and-after timings for a new perf test which just asks rev-list to abbreviate each commit hash (the test repo is linux.git, with commit-graphs): Test origin HEAD ---------------------------------------------------------------------------- 5303.3: rev-list (1) 28.91(28.46+0.44) 29.03(28.65+0.38) +0.4% 5303.4: abbrev-commit (1) 1.18(1.06+0.11) 1.17(1.02+0.14) -0.8% 5303.7: rev-list (50) 28.95(28.56+0.38) 29.50(29.17+0.32) +1.9% 5303.8: abbrev-commit (50) 3.67(3.56+0.10) 3.57(3.42+0.15) -2.7% 5303.11: rev-list (1000) 30.34(29.89+0.43) 30.82(30.35+0.46) +1.6% 5303.12: abbrev-commit (1000) 86.82(86.52+0.29) 77.82(77.59+0.22) -10.4% 5303.15: load 10,000 packs 0.08(0.02+0.05) 0.08(0.02+0.06) +0.0% It doesn't help at all when we have 1 pack (5303.4), but we get a 10% speedup when there are 1000 packs (5303.12). That's a modest speedup for a case that's already slow and we'd hope to avoid in general (note how slow it is even after, because we have to look in each of those packs for abbreviations). But it's a one-line change that clearly matches the original intent, so it seems worth doing. The included perf test may also be useful for keeping an eye on any regressions in the overall abbreviation code. Reported-by: Rasmus Villemoes <[email protected]> Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 47ae905 commit 67bb65d

File tree

2 files changed

+5
-0
lines changed

2 files changed

+5
-0
lines changed

packfile.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,7 @@ unsigned long repo_approximate_object_count(struct repository *r)
923923
count += p->num_objects;
924924
}
925925
r->objects->approximate_object_count = count;
926+
r->objects->approximate_object_count_valid = 1;
926927
}
927928
return r->objects->approximate_object_count;
928929
}

t/perf/p5303-many-packs.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ do
7373
git rev-list --objects --all >/dev/null
7474
'
7575

76+
test_perf "abbrev-commit ($nr_packs)" '
77+
git rev-list --abbrev-commit HEAD >/dev/null
78+
'
79+
7680
# This simulates the interesting part of the repack, which is the
7781
# actual pack generation, without smudging the on-disk setup
7882
# between trials.

0 commit comments

Comments
 (0)