mirror of
https://github.com/git/git
synced 2024-08-28 03:59:25 +00:00
f66e0401ab
This patch fixes an extreme slowdown in pack-objects when you have more
than 1023 packs. See below for numbers.
Since 43fa44fa3b (pack-objects: move in_pack out of struct object_entry,
2018-04-14), we use a complicated system to save some per-object memory.
Each object_entry structs gets a 10-bit field to store the index of the
pack it's in. We map those indices into pointers using
packing_data->in_pack_by_idx, which we initialize at the start of the
program. If we have 2^10 or more packs, then we instead create an array
of pack pointers, one per object. This is packing_data->in_pack.
So far so good. But there's one other tricky case: if a new pack arrives
after we've initialized in_pack_by_idx, it won't have an index yet. We
solve that by calling oe_map_new_pack(), which just switches on the fly
to the less-optimal in_pack mechanism, allocating the array and
back-filling it for already-seen objects.
But that logic kicks in even when we've switched to it already (whether
because we really did see a new pack, or because we had too many packs
in the first place). The result doesn't produce a wrong outcome, but
it's very slow. What happens is this:
- imagine you have a repo with 500k objects and 2000 packs that you
want to repack.
- before looking at any objects, we call prepare_in_pack_by_idx(). It
starts allocating an index for each pack. On the 1024th pack, it
sees there are too many, so it bails, leaving in_pack_by_idx as
NULL.
- while actually adding objects to the packing list, we call
oe_set_in_pack(), which checks whether the pack already has an
index. If it's one of the packs after the first 1023, then it
doesn't have one, and we'll call oe_map_new_pack().
But there's no useful work for that function to do. We're already
using in_pack, so it just uselessly walks over the complete list of
objects, trying to backfill in_pack.
And we end up doing this for almost 1000 packs (each of which may be
triggered by more than one object). And each time it triggers, we
may iterate over up to 500k objects. So in the absolute worst case,
this is quadratic in the number of objects.
The solution is simple: we don't need to bother checking whether the
pack has an index if we've already converted to using in_pack, since by
definition we're not going to use it. So we can just push the "does the
pack have a valid index" check down into that half of the conditional,
where we know we're going to use it.
The current test in p5303 sadly doesn't notice this problem, since it
maxes out at 1000 packs. If we add a new test to it at 2000 packs, it
does show the improvement:
Test HEAD^ HEAD
----------------------------------------------------------------------
5303.12: repack (2000) 26.72(39.68+0.67) 15.70(28.70+0.66) -41.2%
However, these many-pack test cases are rather expensive to run, so
adding larger and larger numbers isn't appealing. Instead, we can show
it off more easily by using GIT_TEST_FULL_IN_PACK_ARRAY, which forces us
into the absolute worst case: no pack has an index, so we'll trigger
oe_map_new_pack() pointlessly for every single object, making it truly
quadratic.
Here are the numbers (on git.git) with the included change to p5303:
Test HEAD^ HEAD
----------------------------------------------------------------------
5303.3: rev-list (1) 2.05(1.98+0.06) 2.06(1.99+0.06) +0.5%
5303.4: repack (1) 33.45(33.46+0.19) 2.75(2.73+0.22) -91.8%
5303.6: rev-list (50) 2.07(2.01+0.06) 2.06(2.01+0.05) -0.5%
5303.7: repack (50) 34.21(35.18+0.16) 3.49(4.50+0.12) -89.8%
5303.9: rev-list (1000) 2.87(2.78+0.08) 2.88(2.80+0.07) +0.3%
5303.10: repack (1000) 41.26(51.30+0.47) 10.75(20.75+0.44) -73.9%
Again, those improvements aren't realistic for the 1-pack case (because
in the real world, the full-array solution doesn't kick in), but it's
more useful to be testing the more-complicated code path.
While we're looking at this issue, we'll tweak one more thing: in
oe_map_new_pack(), we call REALLOC_ARRAY(pack->in_pack). But we'd never
expect to get here unless we're back-filling it for the first time, in
which case it would be NULL. So let's switch that to ALLOC_ARRAY() for
clarity, and add a BUG() to document the expectation. Unfortunately this
code isn't well-covered in the test suite because it's inherently racy
(it only kicks in if somebody else adds a new pack while we're in the
middle of repacking).
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
89 lines
2.4 KiB
Bash
Executable file
89 lines
2.4 KiB
Bash
Executable file
#!/bin/sh
|
|
|
|
test_description='performance with large numbers of packs'
|
|
. ./perf-lib.sh
|
|
|
|
test_perf_large_repo
|
|
|
|
# A real many-pack situation would probably come from having a lot of pushes
|
|
# over time. We don't know how big each push would be, but we can fake it by
|
|
# just walking the first-parent chain and having every 5 commits be their own
|
|
# "push". This isn't _entirely_ accurate, as real pushes would have some
|
|
# duplicate objects due to thin-pack fixing, but it's a reasonable
|
|
# approximation.
|
|
#
|
|
# And then all of the rest of the objects can go in a single packfile that
|
|
# represents the state before any of those pushes (actually, we'll generate
|
|
# that first because in such a setup it would be the oldest pack, and we sort
|
|
# the packs by reverse mtime inside git).
|
|
repack_into_n () {
|
|
rm -rf staging &&
|
|
mkdir staging &&
|
|
|
|
git rev-list --first-parent HEAD |
|
|
sed -n '1~5p' |
|
|
head -n "$1" |
|
|
perl -e 'print reverse <>' \
|
|
>pushes
|
|
|
|
# create base packfile
|
|
head -n 1 pushes |
|
|
git pack-objects --delta-base-offset --revs staging/pack
|
|
|
|
# and then incrementals between each pair of commits
|
|
last= &&
|
|
while read rev
|
|
do
|
|
if test -n "$last"; then
|
|
{
|
|
echo "$rev" &&
|
|
echo "^$last"
|
|
} |
|
|
git pack-objects --delta-base-offset --revs \
|
|
staging/pack || return 1
|
|
fi
|
|
last=$rev
|
|
done <pushes &&
|
|
|
|
# and install the whole thing
|
|
rm -f .git/objects/pack/* &&
|
|
mv staging/* .git/objects/pack/
|
|
}
|
|
|
|
# Pretend we just have a single branch and no reflogs, and that everything is
|
|
# in objects/pack; that makes our fake pack-building via repack_into_n()
|
|
# much simpler.
|
|
test_expect_success 'simplify reachability' '
|
|
tip=$(git rev-parse --verify HEAD) &&
|
|
git for-each-ref --format="option no-deref%0adelete %(refname)" |
|
|
git update-ref --stdin &&
|
|
rm -rf .git/logs &&
|
|
git update-ref refs/heads/master $tip &&
|
|
git symbolic-ref HEAD refs/heads/master &&
|
|
git repack -ad
|
|
'
|
|
|
|
for nr_packs in 1 50 1000
|
|
do
|
|
test_expect_success "create $nr_packs-pack scenario" '
|
|
repack_into_n $nr_packs
|
|
'
|
|
|
|
test_perf "rev-list ($nr_packs)" '
|
|
git rev-list --objects --all >/dev/null
|
|
'
|
|
|
|
# This simulates the interesting part of the repack, which is the
|
|
# actual pack generation, without smudging the on-disk setup
|
|
# between trials.
|
|
test_perf "repack ($nr_packs)" '
|
|
GIT_TEST_FULL_IN_PACK_ARRAY=1 \
|
|
git pack-objects --keep-true-parents \
|
|
--honor-pack-keep --non-empty --all \
|
|
--reflog --indexed-objects --delta-base-offset \
|
|
--stdout </dev/null >/dev/null
|
|
'
|
|
done
|
|
|
|
test_done
|