2013-12-21 22:00:38 +08:00
|
|
|
#!/bin/sh
|
|
|
|
|
|
|
|
|
|
test_description='exercise basic bitmap functionality'
|
|
|
|
|
. ./test-lib.sh
|
|
|
|
|
|
add `ignore_missing_links` mode to revwalk
When pack-objects is computing the reachability bitmap to
serve a fetch request, it can erroneously die() if some of
the UNINTERESTING objects are not present. Upload-pack
throws away HAVE lines from the client for objects we do not
have, but we may have a tip object without all of its
ancestors (e.g., if the tip is no longer reachable and was
new enough to survive a `git prune`, but some of its
reachable objects did get pruned).
In the non-bitmap case, we do a revision walk with the HAVE
objects marked as UNINTERESTING. The revision walker
explicitly ignores errors in accessing UNINTERESTING commits
to handle this case (and we do not bother looking at
UNINTERESTING trees or blobs at all).
When we have bitmaps, however, the process is quite
different. The bitmap index for a pack-objects run is
calculated in two separate steps:
First, we perform an extensive walk from all the HAVEs to
find the full set of objects reachable from them. This walk
is usually optimized away because we are expected to hit an
object with a bitmap during the traversal, which allows us
to terminate early.
Secondly, we perform an extensive walk from all the WANTs,
which usually also terminates early because we hit a commit
with an existing bitmap.
Once we have the resulting bitmaps from the two walks, we
AND-NOT them together to obtain the resulting set of objects
we need to pack.
When we are walking the HAVE objects, the revision walker
does not know that we are walking it only to mark the
results as uninteresting. We strip out the UNINTERESTING flag,
because those objects _are_ interesting to us during the
first walk. We want to keep going to get a complete set of
reachable objects if we can.
We need some way to tell the revision walker that it's OK to
silently truncate the HAVE walk, just like it does for the
UNINTERESTING case. This patch introduces a new
`ignore_missing_links` flag to the `rev_info` struct, which
we set only for the HAVE walk.
It also adds tests to cover UNINTERESTING objects missing
from several positions: a missing blob, a missing tree, and
a missing parent commit. The missing blob already worked (as
we do not care about its contents at all), but the other two
cases caused us to die().
Note that there are a few cases we do not need to test:
1. We do not need to test a missing tree, with the blob
still present. Without the tree that refers to it, we
would not know that the blob is relevant to our walk.
2. We do not need to test a tip commit that is missing.
Upload-pack omits these for us (and in fact, we
complain even in the non-bitmap case if it fails to do
so).
Reported-by: Siddharth Agarwal <sid0@fb.com>
Signed-off-by: Vicent Marti <tanoku@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-28 18:00:43 +08:00
|
|
|
objpath () {
|
|
|
|
|
echo ".git/objects/$(echo "$1" | sed -e 's|\(..\)|\1/|')"
|
|
|
|
|
}
|
|
|
|
|
|
pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
Since 6b8fda2d (pack-objects: use bitmaps when packing objects) there
are two codepaths in pack-objects: with & without using bitmap
reachability index.
However add_object_entry_from_bitmap(), despite its non-bitmapped
counterpart add_object_entry(), in no way does check for whether --local
or --honor-pack-keep or --incremental should be respected. In
non-bitmapped codepath this is handled in want_object_in_pack(), but
bitmapped codepath has simply no such checking at all.
The bitmapped codepath however was allowing to pass in all those options
and with bitmap indices still being used under such conditions -
potentially giving wrong output (e.g. including objects from non-local or
.keep'ed pack).
We can easily fix this by noting the following: when an object comes to
add_object_entry_from_bitmap() it can come for two reasons:
1. entries coming from main pack covered by bitmap index, and
2. object coming from, possibly alternate, loose or other packs.
"2" can be already handled by want_object_in_pack() and to cover
"1" we can teach want_object_in_pack() to expect that *found_pack can be
non-NULL, meaning calling client already found object's pack entry.
In want_object_in_pack() we care to start the checks from already found
pack, if we have one, this way determining the answer right away
in case neither --local nor --honour-pack-keep are active. In
particular, as p5310-pack-bitmaps.sh shows (3 consecutive runs), we do
not do harm to served-with-bitmap clones performance-wise:
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.08(8.20+0.25) 9.09(8.14+0.32) +0.1%
5310.3: simulated clone 1.92(2.12+0.08) 1.93(2.12+0.09) +0.5%
5310.4: simulated fetch 0.82(1.07+0.04) 0.82(1.06+0.04) +0.0%
5310.6: partial bitmap 1.96(2.42+0.13) 1.95(2.40+0.15) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.11(8.16+0.32) 9.11(8.19+0.28) +0.0%
5310.3: simulated clone 1.93(2.14+0.07) 1.92(2.11+0.10) -0.5%
5310.4: simulated fetch 0.82(1.06+0.04) 0.82(1.04+0.05) +0.0%
5310.6: partial bitmap 1.95(2.38+0.16) 1.94(2.39+0.14) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.13(8.17+0.31) 9.07(8.13+0.28) -0.7%
5310.3: simulated clone 1.92(2.13+0.07) 1.91(2.12+0.06) -0.5%
5310.4: simulated fetch 0.82(1.08+0.03) 0.82(1.08+0.03) +0.0%
5310.6: partial bitmap 1.96(2.43+0.14) 1.96(2.42+0.14) +0.0%
with delta timings showing they are all within noise from run to run.
In the general case we do not want to call find_pack_entry_one() more than
once, because it is expensive. This patch splits the loop in
want_object_in_pack() into two parts: finding the object and seeing if it
impacts our choice to include it in the pack. We may call the inexpensive
want_found_object() twice, but we will never call find_pack_entry_one() if we
do not need to.
I appreciate help and discussing this change with Junio C Hamano and
Jeff King.
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-10 23:01:10 +08:00
|
|
|
# show objects present in pack ($1 should be associated *.idx)
|
|
|
|
|
list_packed_objects () {
|
t5310-pack-bitmaps: fix bogus 'pack-objects to file can use bitmap' test
The test 'pack-objects to file can use bitmap' added in 645c432d61
(pack-objects: use reachability bitmap index when generating
non-stdout pack, 2016-09-10) is silently buggy and doesn't check what
it's supposed to.
In 't5310-pack-bitmaps.sh', the 'list_packed_objects' helper function
does what its name implies by running:
git show-index <"$1" | cut -d' ' -f2
The test in question invokes this function like this:
list_packed_objects <packa-$packasha1.idx >packa.objects &&
list_packed_objects <packb-$packbsha1.idx >packb.objects &&
test_cmp packa.objects packb.objects
Note how these two callsites don't specify the name of the pack index
file as the function's parameter, but redirect the function's standard
input from it. This triggers an error message from the shell, as it
has no filename to redirect from in the function, but this error is
ignored, because it happens upstream of a pipe. Consequently, both
invocations produce empty 'pack{a,b}.objects' files, and the
subsequent 'test_cmp' happily finds those two empty files identical.
Fix these two 'list_packed_objects' invocations by specifying the pack
index files as parameters. Furthermore, eliminate the pipe in that
function by replacing it with an &&-chained pair of commands using an
intermediate file, so a failure of 'git show-index' or the shell
redirection will fail the test.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-14 19:47:21 +08:00
|
|
|
git show-index <"$1" >object-list &&
|
|
|
|
|
cut -d' ' -f2 object-list
|
pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
Since 6b8fda2d (pack-objects: use bitmaps when packing objects) there
are two codepaths in pack-objects: with & without using bitmap
reachability index.
However add_object_entry_from_bitmap(), despite its non-bitmapped
counterpart add_object_entry(), in no way does check for whether --local
or --honor-pack-keep or --incremental should be respected. In
non-bitmapped codepath this is handled in want_object_in_pack(), but
bitmapped codepath has simply no such checking at all.
The bitmapped codepath however was allowing to pass in all those options
and with bitmap indices still being used under such conditions -
potentially giving wrong output (e.g. including objects from non-local or
.keep'ed pack).
We can easily fix this by noting the following: when an object comes to
add_object_entry_from_bitmap() it can come for two reasons:
1. entries coming from main pack covered by bitmap index, and
2. object coming from, possibly alternate, loose or other packs.
"2" can be already handled by want_object_in_pack() and to cover
"1" we can teach want_object_in_pack() to expect that *found_pack can be
non-NULL, meaning calling client already found object's pack entry.
In want_object_in_pack() we care to start the checks from already found
pack, if we have one, this way determining the answer right away
in case neither --local nor --honour-pack-keep are active. In
particular, as p5310-pack-bitmaps.sh shows (3 consecutive runs), we do
not do harm to served-with-bitmap clones performance-wise:
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.08(8.20+0.25) 9.09(8.14+0.32) +0.1%
5310.3: simulated clone 1.92(2.12+0.08) 1.93(2.12+0.09) +0.5%
5310.4: simulated fetch 0.82(1.07+0.04) 0.82(1.06+0.04) +0.0%
5310.6: partial bitmap 1.96(2.42+0.13) 1.95(2.40+0.15) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.11(8.16+0.32) 9.11(8.19+0.28) +0.0%
5310.3: simulated clone 1.93(2.14+0.07) 1.92(2.11+0.10) -0.5%
5310.4: simulated fetch 0.82(1.06+0.04) 0.82(1.04+0.05) +0.0%
5310.6: partial bitmap 1.95(2.38+0.16) 1.94(2.39+0.14) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.13(8.17+0.31) 9.07(8.13+0.28) -0.7%
5310.3: simulated clone 1.92(2.13+0.07) 1.91(2.12+0.06) -0.5%
5310.4: simulated fetch 0.82(1.08+0.03) 0.82(1.08+0.03) +0.0%
5310.6: partial bitmap 1.96(2.43+0.14) 1.96(2.42+0.14) +0.0%
with delta timings showing they are all within noise from run to run.
In the general case we do not want to call find_pack_entry_one() more than
once, because it is expensive. This patch splits the loop in
want_object_in_pack() into two parts: finding the object and seeing if it
impacts our choice to include it in the pack. We may call the inexpensive
want_found_object() twice, but we will never call find_pack_entry_one() if we
do not need to.
I appreciate help and discussing this change with Junio C Hamano and
Jeff King.
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-10 23:01:10 +08:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
# has_any pattern-file content-file
|
|
|
|
|
# tests whether content-file has any entry from pattern-file with entries being
|
|
|
|
|
# whole lines.
|
|
|
|
|
has_any () {
|
|
|
|
|
grep -Ff "$1" "$2"
|
|
|
|
|
}
|
|
|
|
|
|
2013-12-21 22:00:38 +08:00
|
|
|
test_expect_success 'setup repo with moderate-sized history' '
|
2019-06-28 17:39:42 +08:00
|
|
|
test_commit_bulk --id=file 100 &&
|
2013-12-21 22:00:38 +08:00
|
|
|
git checkout -b other HEAD~5 &&
|
test-lib: introduce test_commit_bulk
Some tests need to create a string of commits. Doing this with
test_commit is very heavy-weight, as it needs at least one process per
commit (and in fact, uses several).
For bulk creation, we can do much better by using fast-import, but it's
often a pain to generate the input. Let's provide a helper to do so.
We'll use t5310 as a guinea pig, as it has three 10-commit loops. Here
are hyperfine results before and after:
[before]
Benchmark #1: ./t5310-pack-bitmaps.sh --root=/var/ram/git-tests
Time (mean ± σ): 2.846 s ± 0.305 s [User: 3.042 s, System: 0.919 s]
Range (min … max): 2.250 s … 3.210 s 10 runs
[after]
Benchmark #1: ./t5310-pack-bitmaps.sh --root=/var/ram/git-tests
Time (mean ± σ): 2.210 s ± 0.174 s [User: 2.570 s, System: 0.604 s]
Range (min … max): 1.999 s … 2.590 s 10 runs
So we're over 20% faster, while making the callers slightly shorter. We
added a lot more lines in test-lib-function.sh, of course, and the
helper is way more featureful than we need here. But my hope is that it
will be flexible enough to use in more places.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-02 13:16:49 +08:00
|
|
|
test_commit_bulk --id=side 10 &&
|
2013-12-21 22:00:38 +08:00
|
|
|
git checkout master &&
|
pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
Since 6b8fda2d (pack-objects: use bitmaps when packing objects) there
are two codepaths in pack-objects: with & without using bitmap
reachability index.
However add_object_entry_from_bitmap(), despite its non-bitmapped
counterpart add_object_entry(), in no way does check for whether --local
or --honor-pack-keep or --incremental should be respected. In
non-bitmapped codepath this is handled in want_object_in_pack(), but
bitmapped codepath has simply no such checking at all.
The bitmapped codepath however was allowing to pass in all those options
and with bitmap indices still being used under such conditions -
potentially giving wrong output (e.g. including objects from non-local or
.keep'ed pack).
We can easily fix this by noting the following: when an object comes to
add_object_entry_from_bitmap() it can come for two reasons:
1. entries coming from main pack covered by bitmap index, and
2. object coming from, possibly alternate, loose or other packs.
"2" can be already handled by want_object_in_pack() and to cover
"1" we can teach want_object_in_pack() to expect that *found_pack can be
non-NULL, meaning calling client already found object's pack entry.
In want_object_in_pack() we care to start the checks from already found
pack, if we have one, this way determining the answer right away
in case neither --local nor --honour-pack-keep are active. In
particular, as p5310-pack-bitmaps.sh shows (3 consecutive runs), we do
not do harm to served-with-bitmap clones performance-wise:
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.08(8.20+0.25) 9.09(8.14+0.32) +0.1%
5310.3: simulated clone 1.92(2.12+0.08) 1.93(2.12+0.09) +0.5%
5310.4: simulated fetch 0.82(1.07+0.04) 0.82(1.06+0.04) +0.0%
5310.6: partial bitmap 1.96(2.42+0.13) 1.95(2.40+0.15) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.11(8.16+0.32) 9.11(8.19+0.28) +0.0%
5310.3: simulated clone 1.93(2.14+0.07) 1.92(2.11+0.10) -0.5%
5310.4: simulated fetch 0.82(1.06+0.04) 0.82(1.04+0.05) +0.0%
5310.6: partial bitmap 1.95(2.38+0.16) 1.94(2.39+0.14) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.13(8.17+0.31) 9.07(8.13+0.28) -0.7%
5310.3: simulated clone 1.92(2.13+0.07) 1.91(2.12+0.06) -0.5%
5310.4: simulated fetch 0.82(1.08+0.03) 0.82(1.08+0.03) +0.0%
5310.6: partial bitmap 1.96(2.43+0.14) 1.96(2.42+0.14) +0.0%
with delta timings showing they are all within noise from run to run.
In the general case we do not want to call find_pack_entry_one() more than
once, because it is expensive. This patch splits the loop in
want_object_in_pack() into two parts: finding the object and seeing if it
impacts our choice to include it in the pack. We may call the inexpensive
want_found_object() twice, but we will never call find_pack_entry_one() if we
do not need to.
I appreciate help and discussing this change with Junio C Hamano and
Jeff King.
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-10 23:01:10 +08:00
|
|
|
bitmaptip=$(git rev-parse master) &&
|
2013-12-21 22:00:38 +08:00
|
|
|
blob=$(echo tagged-blob | git hash-object -w --stdin) &&
|
|
|
|
|
git tag tagged-blob $blob &&
|
pack-objects: default to writing bitmap hash-cache
Enabling pack.writebitmaphashcache should always be a performance win.
It costs only 4 bytes per object on disk, and the timings in ae4f07fbcc
(pack-bitmap: implement optional name_hash cache, 2013-12-21) show it
improving fetch and partial-bitmap clone times by 40-50%.
The only reason we didn't enable it by default at the time is that early
versions of JGit's bitmap reader complained about the presence of
optional header bits it didn't understand. But that was changed in
JGit's d2fa3987a (Use bitcheck to check for presence of OPT_FULL option,
2013-10-30), which made it into JGit v3.5.0 in late 2014.
So let's turn this option on by default. It's backwards-compatible with
all versions of Git, and if you are also using JGit on the same
repository, you'd only run into problems using a version that's almost 5
years old.
We'll drop the manual setting from all of our test scripts, including
perf tests. This isn't strictly necessary, but it has two advantages:
1. If the hash-cache ever stops being enabled by default, our perf
regression tests will notice.
2. We can use the modified perf tests to show off the behavior of an
otherwise unconfigured repo, as shown below.
These are the results of a few of a perf tests against linux.git that
showed interesting results. You can see the expected speedup in 5310.4,
which was noted in ae4f07fbcc. Curiously, 5310.8 did not improve (and
actually got slower), despite seeing the opposite in ae4f07fbcc.
I don't have an explanation for that.
The tests from p5311 did not exist back then, but do show improvements
(a smaller pack due to better deltas, which we found in less time).
Test HEAD^ HEAD
-------------------------------------------------------------------------------------
5310.4: simulated fetch 7.39(22.70+0.25) 5.64(11.43+0.22) -23.7%
5310.8: clone (partial bitmap) 18.45(24.83+1.19) 19.94(28.40+1.36) +8.1%
5311.31: server (128 days) 0.41(1.13+0.05) 0.34(0.72+0.02) -17.1%
5311.32: size (128 days) 7.4M 7.0M -4.8%
5311.33: client (128 days) 1.33(1.49+0.06) 1.29(1.37+0.12) -3.0%
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-15 14:25:28 +08:00
|
|
|
git config repack.writebitmaps true
|
2013-12-21 22:00:38 +08:00
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'full repack creates bitmaps' '
|
|
|
|
|
git repack -ad &&
|
|
|
|
|
ls .git/objects/pack/ | grep bitmap >output &&
|
|
|
|
|
test_line_count = 1 output
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'rev-list --test-bitmap verifies bitmaps' '
|
|
|
|
|
git rev-list --test-bitmap HEAD
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
rev_list_tests() {
|
|
|
|
|
state=$1
|
|
|
|
|
|
|
|
|
|
test_expect_success "counting commits via bitmap ($state)" '
|
|
|
|
|
git rev-list --count HEAD >expect &&
|
|
|
|
|
git rev-list --use-bitmap-index --count HEAD >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success "counting partial commits via bitmap ($state)" '
|
|
|
|
|
git rev-list --count HEAD~5..HEAD >expect &&
|
|
|
|
|
git rev-list --use-bitmap-index --count HEAD~5..HEAD >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
rev-list: "adjust" results of "--count --use-bitmap-index -n"
If you ask rev-list for:
git rev-list --count --use-bitmap-index HEAD
we optimize out the actual traversal and just give you the
number of bits set in the commit bitmap. This is faster,
which is good.
But if you ask to limit the size of the traversal, like:
git rev-list --count --use-bitmap-index -n 100 HEAD
we'll still output the full bitmapped number we found. On
the surface, that might even seem OK. You explicitly asked
to use the bitmap index, and it was cheap to compute the
real answer, so we gave it to you.
But there's something much more complicated going on under
the hood. If we don't have a bitmap directly for HEAD, then
we have to actually traverse backwards, looking for a
bitmapped commit. And _that_ traversal is bounded by our
`-n` count.
This is a good thing, because it bounds the work we have to
do, which is probably what the user wanted by asking for
`-n`. But now it makes the output quite confusing. You might
get many values:
- your `-n` value, if we walked back and never found a
bitmap (or fewer if there weren't that many commits)
- the actual full count, if we found a bitmap root for
every path of our traversal with in the `-n` limit
- any number in between! We might have walked back and
found _some_ bitmaps, but then cut off the traversal
early with some commits not accounted for in the result.
So you cannot even see a value higher than your `-n` and say
"OK, bitmaps kicked in, this must be the real full count".
The only sane thing is for git to just clamp the value to a
maximum of the `-n` value, which means we should output the
exact same results whether bitmaps are in use or not.
The test in t5310 demonstrates this by using `-n 1`.
Without this patch we fail in the full-bitmap case (where we
do not have to traverse at all) but _not_ in the
partial-bitmap case (where we have to walk down to find an
actual bitmap). With this patch, both cases just work.
I didn't implement the crazy in-between case, just because
it's complicated to set up, and is really a subset of the
full-count case, which we do cover.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-03 15:07:34 +08:00
|
|
|
test_expect_success "counting commits with limit ($state)" '
|
|
|
|
|
git rev-list --count -n 1 HEAD >expect &&
|
|
|
|
|
git rev-list --use-bitmap-index --count -n 1 HEAD >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
2013-12-21 22:00:38 +08:00
|
|
|
test_expect_success "counting non-linear history ($state)" '
|
|
|
|
|
git rev-list --count other...master >expect &&
|
|
|
|
|
git rev-list --use-bitmap-index --count other...master >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
2015-07-02 02:42:17 +08:00
|
|
|
test_expect_success "counting commits with limiting ($state)" '
|
|
|
|
|
git rev-list --count HEAD -- 1.t >expect &&
|
|
|
|
|
git rev-list --use-bitmap-index --count HEAD -- 1.t >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
2020-02-15 02:22:22 +08:00
|
|
|
test_expect_success "counting objects via bitmap ($state)" '
|
|
|
|
|
git rev-list --count --objects HEAD >expect &&
|
|
|
|
|
git rev-list --use-bitmap-index --count --objects HEAD >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
rev-list: allow commit-only bitmap traversals
Ever since we added reachability bitmap support, we've been able to use
it with rev-list to get the full list of objects, like:
git rev-list --objects --use-bitmap-index --all
But you can't do so without --objects, since we weren't ready to just
show the commits. However, the internals of the bitmap code are mostly
ready for this: they avoid opening up trees when walking to fill in the
bitmaps. We just need to actually pass in the rev_info to
traverse_bitmap_commit_list() so it knows which types to bother
triggering our callback for.
For completeness, the perf test now covers both the existing --objects
case, as well as the new commits-only behavior (the objects one got way
faster when we introduced bitmaps, but obviously isn't improved now).
Here are numbers for linux.git:
Test HEAD^ HEAD
------------------------------------------------------------------------
5310.7: rev-list (commits) 8.29(8.10+0.19) 1.76(1.72+0.04) -78.8%
5310.8: rev-list (objects) 8.06(7.94+0.12) 8.14(7.94+0.13) +1.0%
That run was cheating a little, as I didn't have any commit-graph in the
repository, and we'd built it by default these days when running git-gc.
Here are numbers with a commit-graph:
Test HEAD^ HEAD
------------------------------------------------------------------------
5310.7: rev-list (commits) 0.70(0.58+0.12) 0.51(0.46+0.04) -27.1%
5310.8: rev-list (objects) 6.20(6.09+0.10) 6.27(6.16+0.11) +1.1%
Still an improvement, but a lot less impressive.
We could have the perf script remove any commit-graph to show the
out-sized effect, but it probably makes sense to leave it in what would
be a more typical setup.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-15 02:22:27 +08:00
|
|
|
test_expect_success "enumerate commits ($state)" '
|
|
|
|
|
git rev-list --use-bitmap-index HEAD >actual &&
|
|
|
|
|
git rev-list HEAD >expect &&
|
|
|
|
|
test_bitmap_traversal --no-confirm-bitmaps expect actual
|
|
|
|
|
'
|
|
|
|
|
|
2013-12-21 22:00:38 +08:00
|
|
|
test_expect_success "enumerate --objects ($state)" '
|
2020-02-15 02:22:25 +08:00
|
|
|
git rev-list --objects --use-bitmap-index HEAD >actual &&
|
|
|
|
|
git rev-list --objects HEAD >expect &&
|
|
|
|
|
test_bitmap_traversal expect actual
|
2013-12-21 22:00:38 +08:00
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success "bitmap --objects handles non-commit objects ($state)" '
|
|
|
|
|
git rev-list --objects --use-bitmap-index HEAD tagged-blob >actual &&
|
|
|
|
|
grep $blob actual
|
|
|
|
|
'
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
rev_list_tests 'full bitmap'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'clone from bitmapped repository' '
|
|
|
|
|
git clone --no-local --bare . clone.git &&
|
|
|
|
|
git rev-parse HEAD >expect &&
|
|
|
|
|
git --git-dir=clone.git rev-parse HEAD >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
2020-02-15 02:22:41 +08:00
|
|
|
test_expect_success 'partial clone from bitmapped repository' '
|
|
|
|
|
test_config uploadpack.allowfilter true &&
|
|
|
|
|
git clone --no-local --bare --filter=blob:none . partial-clone.git &&
|
|
|
|
|
(
|
|
|
|
|
cd partial-clone.git &&
|
|
|
|
|
pack=$(echo objects/pack/*.pack) &&
|
|
|
|
|
git verify-pack -v "$pack" >have &&
|
|
|
|
|
awk "/blob/ { print \$1 }" <have >blobs &&
|
|
|
|
|
# we expect this single blob because of the direct ref
|
|
|
|
|
git rev-parse refs/tags/tagged-blob >expect &&
|
|
|
|
|
test_cmp expect blobs
|
|
|
|
|
)
|
|
|
|
|
'
|
|
|
|
|
|
2013-12-21 22:00:38 +08:00
|
|
|
test_expect_success 'setup further non-bitmapped commits' '
|
test-lib: introduce test_commit_bulk
Some tests need to create a string of commits. Doing this with
test_commit is very heavy-weight, as it needs at least one process per
commit (and in fact, uses several).
For bulk creation, we can do much better by using fast-import, but it's
often a pain to generate the input. Let's provide a helper to do so.
We'll use t5310 as a guinea pig, as it has three 10-commit loops. Here
are hyperfine results before and after:
[before]
Benchmark #1: ./t5310-pack-bitmaps.sh --root=/var/ram/git-tests
Time (mean ± σ): 2.846 s ± 0.305 s [User: 3.042 s, System: 0.919 s]
Range (min … max): 2.250 s … 3.210 s 10 runs
[after]
Benchmark #1: ./t5310-pack-bitmaps.sh --root=/var/ram/git-tests
Time (mean ± σ): 2.210 s ± 0.174 s [User: 2.570 s, System: 0.604 s]
Range (min … max): 1.999 s … 2.590 s 10 runs
So we're over 20% faster, while making the callers slightly shorter. We
added a lot more lines in test-lib-function.sh, of course, and the
helper is way more featureful than we need here. But my hope is that it
will be flexible enough to use in more places.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-02 13:16:49 +08:00
|
|
|
test_commit_bulk --id=further 10
|
2013-12-21 22:00:38 +08:00
|
|
|
'
|
|
|
|
|
|
|
|
|
|
rev_list_tests 'partial bitmap'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'fetch (partial bitmap)' '
|
|
|
|
|
git --git-dir=clone.git fetch origin master:master &&
|
|
|
|
|
git rev-parse HEAD >expect &&
|
|
|
|
|
git --git-dir=clone.git rev-parse HEAD >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
2016-12-29 06:45:42 +08:00
|
|
|
test_expect_success 'incremental repack fails when bitmaps are requested' '
|
2013-12-21 22:00:38 +08:00
|
|
|
test_commit more-1 &&
|
2016-12-29 06:45:42 +08:00
|
|
|
test_must_fail git repack -d 2>err &&
|
|
|
|
|
test_i18ngrep "Incremental repacks are incompatible with bitmap" err
|
2013-12-21 22:00:38 +08:00
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'incremental repack can disable bitmaps' '
|
|
|
|
|
test_commit more-2 &&
|
|
|
|
|
git repack -d --no-write-bitmap-index
|
|
|
|
|
'
|
|
|
|
|
|
pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
Since 6b8fda2d (pack-objects: use bitmaps when packing objects) there
are two codepaths in pack-objects: with & without using bitmap
reachability index.
However add_object_entry_from_bitmap(), despite its non-bitmapped
counterpart add_object_entry(), in no way does check for whether --local
or --honor-pack-keep or --incremental should be respected. In
non-bitmapped codepath this is handled in want_object_in_pack(), but
bitmapped codepath has simply no such checking at all.
The bitmapped codepath however was allowing to pass in all those options
and with bitmap indices still being used under such conditions -
potentially giving wrong output (e.g. including objects from non-local or
.keep'ed pack).
We can easily fix this by noting the following: when an object comes to
add_object_entry_from_bitmap() it can come for two reasons:
1. entries coming from main pack covered by bitmap index, and
2. object coming from, possibly alternate, loose or other packs.
"2" can be already handled by want_object_in_pack() and to cover
"1" we can teach want_object_in_pack() to expect that *found_pack can be
non-NULL, meaning calling client already found object's pack entry.
In want_object_in_pack() we care to start the checks from already found
pack, if we have one, this way determining the answer right away
in case neither --local nor --honour-pack-keep are active. In
particular, as p5310-pack-bitmaps.sh shows (3 consecutive runs), we do
not do harm to served-with-bitmap clones performance-wise:
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.08(8.20+0.25) 9.09(8.14+0.32) +0.1%
5310.3: simulated clone 1.92(2.12+0.08) 1.93(2.12+0.09) +0.5%
5310.4: simulated fetch 0.82(1.07+0.04) 0.82(1.06+0.04) +0.0%
5310.6: partial bitmap 1.96(2.42+0.13) 1.95(2.40+0.15) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.11(8.16+0.32) 9.11(8.19+0.28) +0.0%
5310.3: simulated clone 1.93(2.14+0.07) 1.92(2.11+0.10) -0.5%
5310.4: simulated fetch 0.82(1.06+0.04) 0.82(1.04+0.05) +0.0%
5310.6: partial bitmap 1.95(2.38+0.16) 1.94(2.39+0.14) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.13(8.17+0.31) 9.07(8.13+0.28) -0.7%
5310.3: simulated clone 1.92(2.13+0.07) 1.91(2.12+0.06) -0.5%
5310.4: simulated fetch 0.82(1.08+0.03) 0.82(1.08+0.03) +0.0%
5310.6: partial bitmap 1.96(2.43+0.14) 1.96(2.42+0.14) +0.0%
with delta timings showing they are all within noise from run to run.
In the general case we do not want to call find_pack_entry_one() more than
once, because it is expensive. This patch splits the loop in
want_object_in_pack() into two parts: finding the object and seeing if it
impacts our choice to include it in the pack. We may call the inexpensive
want_found_object() twice, but we will never call find_pack_entry_one() if we
do not need to.
I appreciate help and discussing this change with Junio C Hamano and
Jeff King.
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-10 23:01:10 +08:00
|
|
|
test_expect_success 'pack-objects respects --local (non-local loose)' '
|
|
|
|
|
git init --bare alt.git &&
|
|
|
|
|
echo $(pwd)/alt.git/objects >.git/objects/info/alternates &&
|
|
|
|
|
echo content1 >file1 &&
|
|
|
|
|
# non-local loose object which is not present in bitmapped pack
|
|
|
|
|
altblob=$(GIT_DIR=alt.git git hash-object -w file1) &&
|
|
|
|
|
# non-local loose object which is also present in bitmapped pack
|
|
|
|
|
git cat-file blob $blob | GIT_DIR=alt.git git hash-object -w --stdin &&
|
|
|
|
|
git add file1 &&
|
|
|
|
|
test_tick &&
|
|
|
|
|
git commit -m commit_file1 &&
|
|
|
|
|
echo HEAD | git pack-objects --local --stdout --revs >1.pack &&
|
|
|
|
|
git index-pack 1.pack &&
|
|
|
|
|
list_packed_objects 1.idx >1.objects &&
|
|
|
|
|
printf "%s\n" "$altblob" "$blob" >nonlocal-loose &&
|
|
|
|
|
! has_any nonlocal-loose 1.objects
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack-objects respects --honor-pack-keep (local non-bitmapped pack)' '
|
|
|
|
|
echo content2 >file2 &&
|
|
|
|
|
blob2=$(git hash-object -w file2) &&
|
|
|
|
|
git add file2 &&
|
|
|
|
|
test_tick &&
|
|
|
|
|
git commit -m commit_file2 &&
|
|
|
|
|
printf "%s\n" "$blob2" "$bitmaptip" >keepobjects &&
|
|
|
|
|
pack2=$(git pack-objects pack2 <keepobjects) &&
|
|
|
|
|
mv pack2-$pack2.* .git/objects/pack/ &&
|
|
|
|
|
>.git/objects/pack/pack2-$pack2.keep &&
|
|
|
|
|
rm $(objpath $blob2) &&
|
|
|
|
|
echo HEAD | git pack-objects --honor-pack-keep --stdout --revs >2a.pack &&
|
|
|
|
|
git index-pack 2a.pack &&
|
|
|
|
|
list_packed_objects 2a.idx >2a.objects &&
|
|
|
|
|
! has_any keepobjects 2a.objects
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack-objects respects --local (non-local pack)' '
|
|
|
|
|
mv .git/objects/pack/pack2-$pack2.* alt.git/objects/pack/ &&
|
|
|
|
|
echo HEAD | git pack-objects --local --stdout --revs >2b.pack &&
|
|
|
|
|
git index-pack 2b.pack &&
|
|
|
|
|
list_packed_objects 2b.idx >2b.objects &&
|
|
|
|
|
! has_any keepobjects 2b.objects
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack-objects respects --honor-pack-keep (local bitmapped pack)' '
|
|
|
|
|
ls .git/objects/pack/ | grep bitmap >output &&
|
|
|
|
|
test_line_count = 1 output &&
|
|
|
|
|
packbitmap=$(basename $(cat output) .bitmap) &&
|
|
|
|
|
list_packed_objects .git/objects/pack/$packbitmap.idx >packbitmap.objects &&
|
|
|
|
|
test_when_finished "rm -f .git/objects/pack/$packbitmap.keep" &&
|
|
|
|
|
>.git/objects/pack/$packbitmap.keep &&
|
|
|
|
|
echo HEAD | git pack-objects --honor-pack-keep --stdout --revs >3a.pack &&
|
|
|
|
|
git index-pack 3a.pack &&
|
|
|
|
|
list_packed_objects 3a.idx >3a.objects &&
|
|
|
|
|
! has_any packbitmap.objects 3a.objects
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack-objects respects --local (non-local bitmapped pack)' '
|
|
|
|
|
mv .git/objects/pack/$packbitmap.* alt.git/objects/pack/ &&
|
2018-10-13 01:34:20 +08:00
|
|
|
rm -f .git/objects/pack/multi-pack-index &&
|
pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
Since 6b8fda2d (pack-objects: use bitmaps when packing objects) there
are two codepaths in pack-objects: with & without using bitmap
reachability index.
However add_object_entry_from_bitmap(), despite its non-bitmapped
counterpart add_object_entry(), in no way does check for whether --local
or --honor-pack-keep or --incremental should be respected. In
non-bitmapped codepath this is handled in want_object_in_pack(), but
bitmapped codepath has simply no such checking at all.
The bitmapped codepath however was allowing to pass in all those options
and with bitmap indices still being used under such conditions -
potentially giving wrong output (e.g. including objects from non-local or
.keep'ed pack).
We can easily fix this by noting the following: when an object comes to
add_object_entry_from_bitmap() it can come for two reasons:
1. entries coming from main pack covered by bitmap index, and
2. object coming from, possibly alternate, loose or other packs.
"2" can be already handled by want_object_in_pack() and to cover
"1" we can teach want_object_in_pack() to expect that *found_pack can be
non-NULL, meaning calling client already found object's pack entry.
In want_object_in_pack() we care to start the checks from already found
pack, if we have one, this way determining the answer right away
in case neither --local nor --honour-pack-keep are active. In
particular, as p5310-pack-bitmaps.sh shows (3 consecutive runs), we do
not do harm to served-with-bitmap clones performance-wise:
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.08(8.20+0.25) 9.09(8.14+0.32) +0.1%
5310.3: simulated clone 1.92(2.12+0.08) 1.93(2.12+0.09) +0.5%
5310.4: simulated fetch 0.82(1.07+0.04) 0.82(1.06+0.04) +0.0%
5310.6: partial bitmap 1.96(2.42+0.13) 1.95(2.40+0.15) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.11(8.16+0.32) 9.11(8.19+0.28) +0.0%
5310.3: simulated clone 1.93(2.14+0.07) 1.92(2.11+0.10) -0.5%
5310.4: simulated fetch 0.82(1.06+0.04) 0.82(1.04+0.05) +0.0%
5310.6: partial bitmap 1.95(2.38+0.16) 1.94(2.39+0.14) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.13(8.17+0.31) 9.07(8.13+0.28) -0.7%
5310.3: simulated clone 1.92(2.13+0.07) 1.91(2.12+0.06) -0.5%
5310.4: simulated fetch 0.82(1.08+0.03) 0.82(1.08+0.03) +0.0%
5310.6: partial bitmap 1.96(2.43+0.14) 1.96(2.42+0.14) +0.0%
with delta timings showing they are all within noise from run to run.
In the general case we do not want to call find_pack_entry_one() more than
once, because it is expensive. This patch splits the loop in
want_object_in_pack() into two parts: finding the object and seeing if it
impacts our choice to include it in the pack. We may call the inexpensive
want_found_object() twice, but we will never call find_pack_entry_one() if we
do not need to.
I appreciate help and discussing this change with Junio C Hamano and
Jeff King.
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-10 23:01:10 +08:00
|
|
|
test_when_finished "mv alt.git/objects/pack/$packbitmap.* .git/objects/pack/" &&
|
|
|
|
|
echo HEAD | git pack-objects --local --stdout --revs >3b.pack &&
|
|
|
|
|
git index-pack 3b.pack &&
|
|
|
|
|
list_packed_objects 3b.idx >3b.objects &&
|
|
|
|
|
! has_any packbitmap.objects 3b.objects
|
|
|
|
|
'
|
|
|
|
|
|
pack-objects: use reachability bitmap index when generating non-stdout pack
Starting from 6b8fda2d (pack-objects: use bitmaps when packing objects)
if a repository has bitmap index, pack-objects can nicely speedup
"Counting objects" graph traversal phase. That however was done only for
case when resultant pack is sent to stdout, not written into a file.
The reason here is for on-disk repack by default we want:
- to produce good pack (with bitmap index not-yet-packed objects are
emitted to pack in suboptimal order).
- to use more robust pack-generation codepath (avoiding possible
bugs in bitmap code and possible bitmap index corruption).
Jeff King further explains:
The reason for this split is that pack-objects tries to determine how
"careful" it should be based on whether we are packing to disk or to
stdout. Packing to disk implies "git repack", and that we will likely
delete the old packs after finishing. We want to be more careful (so
as not to carry forward a corruption, and to generate a more optimal
pack), and we presumably run less frequently and can afford extra CPU.
Whereas packing to stdout implies serving a remote via "git fetch" or
"git push". This happens more frequently (e.g., a server handling many
fetching clients), and we assume the receiving end takes more
responsibility for verifying the data.
But this isn't always the case. One might want to generate on-disk
packfiles for a specialized object transfer. Just using "--stdout" and
writing to a file is not optimal, as it will not generate the matching
pack index.
So it would be useful to have some way of overriding this heuristic:
to tell pack-objects that even though it should generate on-disk
files, it is still OK to use the reachability bitmaps to do the
traversal.
So we can teach pack-objects to use bitmap index for initial object
counting phase when generating resultant pack file too:
- if we take care to not let it be activated under git-repack:
See above about repack robustness and not forward-carrying corruption.
- if we know bitmap index generation is not enabled for resultant pack:
The current code has singleton bitmap_git, so it cannot work
simultaneously with two bitmap indices.
We also want to avoid (at least with current implementation)
generating bitmaps off of bitmaps. The reason here is: when generating
a pack, not-yet-packed objects will be emitted into pack in
suboptimal order and added to tail of the bitmap as "extended entries".
When the resultant pack + some new objects in associated repository
are in turn used to generate another pack with bitmap, the situation
repeats: new objects are again not emitted optimally and just added to
bitmap tail - not in recency order.
So the pack badness can grow over time when at each step we have
bitmapped pack + some other objects. That's why we want to avoid
generating bitmaps off of bitmaps, not to let pack badness grow.
- if we keep pack reuse enabled still only for "send-to-stdout" case:
Because pack-to-file needs to generate index for destination pack, and
currently on pack reuse raw entries are directly written out to the
destination pack by write_reused_pack(), bypassing needed for pack index
generation bookkeeping done by regular codepath in write_one() and
friends.
( In the future we might teach pack-reuse code about cases when index
also needs to be generated for resultant pack and remove
pack-reuse-only-for-stdout limitation )
This way for pack-objects -> file we get nice speedup:
erp5.git[1] (~230MB) extracted from ~ 5GB lab.nexedi.com backup
repository managed by git-backup[2] via
time echo 0186ac99 | git pack-objects --revs erp5pack
before: 37.2s
after: 26.2s
And for `git repack -adb` packed git.git
time echo 5c589a73 | git pack-objects --revs gitpack
before: 7.1s
after: 3.6s
i.e. it can be 30% - 50% speedup for pack extraction.
git-backup extracts many packs on repositories restoration. That was my
initial motivation for the patch.
[1] https://lab.nexedi.com/nexedi/erp5
[2] https://lab.nexedi.com/kirr/git-backup
NOTE
Jeff also suggests that pack.useBitmaps was probably a mistake to
introduce originally. This way we are not adding another config point,
but instead just always default to-file pack-objects not to use bitmap
index: Tools which need to generate on-disk packs with using bitmap, can
pass --use-bitmap-index explicitly. And git-repack does never pass
--use-bitmap-index, so this way we can be sure regular on-disk repacking
remains robust.
NOTE2
`git pack-objects --stdout >file.pack` + `git index-pack file.pack` is much slower
than `git pack-objects file.pack`. Extracting erp5.git pack from
lab.nexedi.com backup repository:
$ time echo 0186ac99 | git pack-objects --stdout --revs >erp5pack-stdout.pack
real 0m22.309s
user 0m21.148s
sys 0m0.932s
$ time git index-pack erp5pack-stdout.pack
real 0m50.873s <-- more than 2 times slower than time to generate pack itself!
user 0m49.300s
sys 0m1.360s
So the time for
`pack-object --stdout >file.pack` + `index-pack file.pack` is 72s,
while
`pack-objects file.pack` which does both pack and index is 27s.
And even
`pack-objects --no-use-bitmap-index file.pack` is 37s.
Jeff explains:
The packfile does not carry the sha1 of the objects. A receiving
index-pack has to compute them itself, including inflating and applying
all of the deltas.
that's why for `git-backup restore` we want to teach `git pack-objects
file.pack` to use bitmaps instead of using `git pack-objects --stdout
>file.pack` + `git index-pack file.pack`.
NOTE3
The speedup is now tracked via t/perf/p5310-pack-bitmaps.sh
Test 56dfeb62 this tree
--------------------------------------------------------------------------------
5310.2: repack to disk 8.98(8.05+0.29) 9.05(8.08+0.33) +0.8%
5310.3: simulated clone 2.02(2.27+0.09) 2.01(2.25+0.08) -0.5%
5310.4: simulated fetch 0.81(1.07+0.02) 0.81(1.05+0.04) +0.0%
5310.5: pack to file 7.58(7.04+0.28) 7.60(7.04+0.30) +0.3%
5310.6: pack to file (bitmap) 7.55(7.02+0.28) 3.25(2.82+0.18) -57.0%
5310.8: clone (partial bitmap) 1.83(2.26+0.12) 1.82(2.22+0.14) -0.5%
5310.9: pack to file (partial bitmap) 6.86(6.58+0.30) 2.87(2.74+0.20) -58.2%
More context:
http://marc.info/?t=146792101400001&r=1&w=2
http://public-inbox.org/git/20160707190917.20011-1-kirr@nexedi.com/T/#t
Cc: Vicent Marti <tanoku@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-10 23:01:44 +08:00
|
|
|
test_expect_success 'pack-objects to file can use bitmap' '
|
|
|
|
|
# make sure we still have 1 bitmap index from previous tests
|
|
|
|
|
ls .git/objects/pack/ | grep bitmap >output &&
|
|
|
|
|
test_line_count = 1 output &&
|
|
|
|
|
# verify equivalent packs are generated with/without using bitmap index
|
|
|
|
|
packasha1=$(git pack-objects --no-use-bitmap-index --all packa </dev/null) &&
|
|
|
|
|
packbsha1=$(git pack-objects --use-bitmap-index --all packb </dev/null) &&
|
t5310-pack-bitmaps: fix bogus 'pack-objects to file can use bitmap' test
The test 'pack-objects to file can use bitmap' added in 645c432d61
(pack-objects: use reachability bitmap index when generating
non-stdout pack, 2016-09-10) is silently buggy and doesn't check what
it's supposed to.
In 't5310-pack-bitmaps.sh', the 'list_packed_objects' helper function
does what its name implies by running:
git show-index <"$1" | cut -d' ' -f2
The test in question invokes this function like this:
list_packed_objects <packa-$packasha1.idx >packa.objects &&
list_packed_objects <packb-$packbsha1.idx >packb.objects &&
test_cmp packa.objects packb.objects
Note how these two callsites don't specify the name of the pack index
file as the function's parameter, but redirect the function's standard
input from it. This triggers an error message from the shell, as it
has no filename to redirect from in the function, but this error is
ignored, because it happens upstream of a pipe. Consequently, both
invocations produce empty 'pack{a,b}.objects' files, and the
subsequent 'test_cmp' happily finds those two empty files identical.
Fix these two 'list_packed_objects' invocations by specifying the pack
index files as parameters. Furthermore, eliminate the pipe in that
function by replacing it with an &&-chained pair of commands using an
intermediate file, so a failure of 'git show-index' or the shell
redirection will fail the test.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-14 19:47:21 +08:00
|
|
|
list_packed_objects packa-$packasha1.idx >packa.objects &&
|
|
|
|
|
list_packed_objects packb-$packbsha1.idx >packb.objects &&
|
pack-objects: use reachability bitmap index when generating non-stdout pack
Starting from 6b8fda2d (pack-objects: use bitmaps when packing objects)
if a repository has bitmap index, pack-objects can nicely speedup
"Counting objects" graph traversal phase. That however was done only for
case when resultant pack is sent to stdout, not written into a file.
The reason here is for on-disk repack by default we want:
- to produce good pack (with bitmap index not-yet-packed objects are
emitted to pack in suboptimal order).
- to use more robust pack-generation codepath (avoiding possible
bugs in bitmap code and possible bitmap index corruption).
Jeff King further explains:
The reason for this split is that pack-objects tries to determine how
"careful" it should be based on whether we are packing to disk or to
stdout. Packing to disk implies "git repack", and that we will likely
delete the old packs after finishing. We want to be more careful (so
as not to carry forward a corruption, and to generate a more optimal
pack), and we presumably run less frequently and can afford extra CPU.
Whereas packing to stdout implies serving a remote via "git fetch" or
"git push". This happens more frequently (e.g., a server handling many
fetching clients), and we assume the receiving end takes more
responsibility for verifying the data.
But this isn't always the case. One might want to generate on-disk
packfiles for a specialized object transfer. Just using "--stdout" and
writing to a file is not optimal, as it will not generate the matching
pack index.
So it would be useful to have some way of overriding this heuristic:
to tell pack-objects that even though it should generate on-disk
files, it is still OK to use the reachability bitmaps to do the
traversal.
So we can teach pack-objects to use bitmap index for initial object
counting phase when generating resultant pack file too:
- if we take care to not let it be activated under git-repack:
See above about repack robustness and not forward-carrying corruption.
- if we know bitmap index generation is not enabled for resultant pack:
The current code has singleton bitmap_git, so it cannot work
simultaneously with two bitmap indices.
We also want to avoid (at least with current implementation)
generating bitmaps off of bitmaps. The reason here is: when generating
a pack, not-yet-packed objects will be emitted into pack in
suboptimal order and added to tail of the bitmap as "extended entries".
When the resultant pack + some new objects in associated repository
are in turn used to generate another pack with bitmap, the situation
repeats: new objects are again not emitted optimally and just added to
bitmap tail - not in recency order.
So the pack badness can grow over time when at each step we have
bitmapped pack + some other objects. That's why we want to avoid
generating bitmaps off of bitmaps, not to let pack badness grow.
- if we keep pack reuse enabled still only for "send-to-stdout" case:
Because pack-to-file needs to generate index for destination pack, and
currently on pack reuse raw entries are directly written out to the
destination pack by write_reused_pack(), bypassing needed for pack index
generation bookkeeping done by regular codepath in write_one() and
friends.
( In the future we might teach pack-reuse code about cases when index
also needs to be generated for resultant pack and remove
pack-reuse-only-for-stdout limitation )
This way for pack-objects -> file we get nice speedup:
erp5.git[1] (~230MB) extracted from ~ 5GB lab.nexedi.com backup
repository managed by git-backup[2] via
time echo 0186ac99 | git pack-objects --revs erp5pack
before: 37.2s
after: 26.2s
And for `git repack -adb` packed git.git
time echo 5c589a73 | git pack-objects --revs gitpack
before: 7.1s
after: 3.6s
i.e. it can be 30% - 50% speedup for pack extraction.
git-backup extracts many packs on repositories restoration. That was my
initial motivation for the patch.
[1] https://lab.nexedi.com/nexedi/erp5
[2] https://lab.nexedi.com/kirr/git-backup
NOTE
Jeff also suggests that pack.useBitmaps was probably a mistake to
introduce originally. This way we are not adding another config point,
but instead just always default to-file pack-objects not to use bitmap
index: Tools which need to generate on-disk packs with using bitmap, can
pass --use-bitmap-index explicitly. And git-repack does never pass
--use-bitmap-index, so this way we can be sure regular on-disk repacking
remains robust.
NOTE2
`git pack-objects --stdout >file.pack` + `git index-pack file.pack` is much slower
than `git pack-objects file.pack`. Extracting erp5.git pack from
lab.nexedi.com backup repository:
$ time echo 0186ac99 | git pack-objects --stdout --revs >erp5pack-stdout.pack
real 0m22.309s
user 0m21.148s
sys 0m0.932s
$ time git index-pack erp5pack-stdout.pack
real 0m50.873s <-- more than 2 times slower than time to generate pack itself!
user 0m49.300s
sys 0m1.360s
So the time for
`pack-object --stdout >file.pack` + `index-pack file.pack` is 72s,
while
`pack-objects file.pack` which does both pack and index is 27s.
And even
`pack-objects --no-use-bitmap-index file.pack` is 37s.
Jeff explains:
The packfile does not carry the sha1 of the objects. A receiving
index-pack has to compute them itself, including inflating and applying
all of the deltas.
that's why for `git-backup restore` we want to teach `git pack-objects
file.pack` to use bitmaps instead of using `git pack-objects --stdout
>file.pack` + `git index-pack file.pack`.
NOTE3
The speedup is now tracked via t/perf/p5310-pack-bitmaps.sh
Test 56dfeb62 this tree
--------------------------------------------------------------------------------
5310.2: repack to disk 8.98(8.05+0.29) 9.05(8.08+0.33) +0.8%
5310.3: simulated clone 2.02(2.27+0.09) 2.01(2.25+0.08) -0.5%
5310.4: simulated fetch 0.81(1.07+0.02) 0.81(1.05+0.04) +0.0%
5310.5: pack to file 7.58(7.04+0.28) 7.60(7.04+0.30) +0.3%
5310.6: pack to file (bitmap) 7.55(7.02+0.28) 3.25(2.82+0.18) -57.0%
5310.8: clone (partial bitmap) 1.83(2.26+0.12) 1.82(2.22+0.14) -0.5%
5310.9: pack to file (partial bitmap) 6.86(6.58+0.30) 2.87(2.74+0.20) -58.2%
More context:
http://marc.info/?t=146792101400001&r=1&w=2
http://public-inbox.org/git/20160707190917.20011-1-kirr@nexedi.com/T/#t
Cc: Vicent Marti <tanoku@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-10 23:01:44 +08:00
|
|
|
test_cmp packa.objects packb.objects
|
|
|
|
|
'
|
|
|
|
|
|
2013-12-21 22:00:38 +08:00
|
|
|
test_expect_success 'full repack, reusing previous bitmaps' '
|
|
|
|
|
git repack -ad &&
|
|
|
|
|
ls .git/objects/pack/ | grep bitmap >output &&
|
|
|
|
|
test_line_count = 1 output
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'fetch (full bitmap)' '
|
|
|
|
|
git --git-dir=clone.git fetch origin master:master &&
|
|
|
|
|
git rev-parse HEAD >expect &&
|
|
|
|
|
git --git-dir=clone.git rev-parse HEAD >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
add `ignore_missing_links` mode to revwalk
When pack-objects is computing the reachability bitmap to
serve a fetch request, it can erroneously die() if some of
the UNINTERESTING objects are not present. Upload-pack
throws away HAVE lines from the client for objects we do not
have, but we may have a tip object without all of its
ancestors (e.g., if the tip is no longer reachable and was
new enough to survive a `git prune`, but some of its
reachable objects did get pruned).
In the non-bitmap case, we do a revision walk with the HAVE
objects marked as UNINTERESTING. The revision walker
explicitly ignores errors in accessing UNINTERESTING commits
to handle this case (and we do not bother looking at
UNINTERESTING trees or blobs at all).
When we have bitmaps, however, the process is quite
different. The bitmap index for a pack-objects run is
calculated in two separate steps:
First, we perform an extensive walk from all the HAVEs to
find the full set of objects reachable from them. This walk
is usually optimized away because we are expected to hit an
object with a bitmap during the traversal, which allows us
to terminate early.
Secondly, we perform an extensive walk from all the WANTs,
which usually also terminates early because we hit a commit
with an existing bitmap.
Once we have the resulting bitmaps from the two walks, we
AND-NOT them together to obtain the resulting set of objects
we need to pack.
When we are walking the HAVE objects, the revision walker
does not know that we are walking it only to mark the
results as uninteresting. We strip out the UNINTERESTING flag,
because those objects _are_ interesting to us during the
first walk. We want to keep going to get a complete set of
reachable objects if we can.
We need some way to tell the revision walker that it's OK to
silently truncate the HAVE walk, just like it does for the
UNINTERESTING case. This patch introduces a new
`ignore_missing_links` flag to the `rev_info` struct, which
we set only for the HAVE walk.
It also adds tests to cover UNINTERESTING objects missing
from several positions: a missing blob, a missing tree, and
a missing parent commit. The missing blob already worked (as
we do not care about its contents at all), but the other two
cases caused us to die().
Note that there are a few cases we do not need to test:
1. We do not need to test a missing tree, with the blob
still present. Without the tree that refers to it, we
would not know that the blob is relevant to our walk.
2. We do not need to test a tip commit that is missing.
Upload-pack omits these for us (and in fact, we
complain even in the non-bitmap case if it fails to do
so).
Reported-by: Siddharth Agarwal <sid0@fb.com>
Signed-off-by: Vicent Marti <tanoku@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-28 18:00:43 +08:00
|
|
|
test_expect_success 'create objects for missing-HAVE tests' '
|
|
|
|
|
blob=$(echo "missing have" | git hash-object -w --stdin) &&
|
|
|
|
|
tree=$(printf "100644 blob $blob\tfile\n" | git mktree) &&
|
|
|
|
|
parent=$(echo parent | git commit-tree $tree) &&
|
|
|
|
|
commit=$(echo commit | git commit-tree $tree -p $parent) &&
|
|
|
|
|
cat >revs <<-EOF
|
|
|
|
|
HEAD
|
|
|
|
|
^HEAD^
|
|
|
|
|
^$commit
|
|
|
|
|
EOF
|
|
|
|
|
'
|
|
|
|
|
|
pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
Since 6b8fda2d (pack-objects: use bitmaps when packing objects) there
are two codepaths in pack-objects: with & without using bitmap
reachability index.
However add_object_entry_from_bitmap(), despite its non-bitmapped
counterpart add_object_entry(), in no way does check for whether --local
or --honor-pack-keep or --incremental should be respected. In
non-bitmapped codepath this is handled in want_object_in_pack(), but
bitmapped codepath has simply no such checking at all.
The bitmapped codepath however was allowing to pass in all those options
and with bitmap indices still being used under such conditions -
potentially giving wrong output (e.g. including objects from non-local or
.keep'ed pack).
We can easily fix this by noting the following: when an object comes to
add_object_entry_from_bitmap() it can come for two reasons:
1. entries coming from main pack covered by bitmap index, and
2. object coming from, possibly alternate, loose or other packs.
"2" can be already handled by want_object_in_pack() and to cover
"1" we can teach want_object_in_pack() to expect that *found_pack can be
non-NULL, meaning calling client already found object's pack entry.
In want_object_in_pack() we care to start the checks from already found
pack, if we have one, this way determining the answer right away
in case neither --local nor --honour-pack-keep are active. In
particular, as p5310-pack-bitmaps.sh shows (3 consecutive runs), we do
not do harm to served-with-bitmap clones performance-wise:
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.08(8.20+0.25) 9.09(8.14+0.32) +0.1%
5310.3: simulated clone 1.92(2.12+0.08) 1.93(2.12+0.09) +0.5%
5310.4: simulated fetch 0.82(1.07+0.04) 0.82(1.06+0.04) +0.0%
5310.6: partial bitmap 1.96(2.42+0.13) 1.95(2.40+0.15) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.11(8.16+0.32) 9.11(8.19+0.28) +0.0%
5310.3: simulated clone 1.93(2.14+0.07) 1.92(2.11+0.10) -0.5%
5310.4: simulated fetch 0.82(1.06+0.04) 0.82(1.04+0.05) +0.0%
5310.6: partial bitmap 1.95(2.38+0.16) 1.94(2.39+0.14) -0.5%
Test 56dfeb62 this tree
-----------------------------------------------------------------
5310.2: repack to disk 9.13(8.17+0.31) 9.07(8.13+0.28) -0.7%
5310.3: simulated clone 1.92(2.13+0.07) 1.91(2.12+0.06) -0.5%
5310.4: simulated fetch 0.82(1.08+0.03) 0.82(1.08+0.03) +0.0%
5310.6: partial bitmap 1.96(2.43+0.14) 1.96(2.42+0.14) +0.0%
with delta timings showing they are all within noise from run to run.
In the general case we do not want to call find_pack_entry_one() more than
once, because it is expensive. This patch splits the loop in
want_object_in_pack() into two parts: finding the object and seeing if it
impacts our choice to include it in the pack. We may call the inexpensive
want_found_object() twice, but we will never call find_pack_entry_one() if we
do not need to.
I appreciate help and discussing this change with Junio C Hamano and
Jeff King.
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-10 23:01:10 +08:00
|
|
|
test_expect_success 'pack-objects respects --incremental' '
|
|
|
|
|
cat >revs2 <<-EOF &&
|
|
|
|
|
HEAD
|
|
|
|
|
$commit
|
|
|
|
|
EOF
|
|
|
|
|
git pack-objects --incremental --stdout --revs <revs2 >4.pack &&
|
|
|
|
|
git index-pack 4.pack &&
|
|
|
|
|
list_packed_objects 4.idx >4.objects &&
|
|
|
|
|
test_line_count = 4 4.objects &&
|
|
|
|
|
git rev-list --objects $commit >revlist &&
|
|
|
|
|
cut -d" " -f1 revlist |sort >objects &&
|
|
|
|
|
test_cmp 4.objects objects
|
|
|
|
|
'
|
|
|
|
|
|
add `ignore_missing_links` mode to revwalk
When pack-objects is computing the reachability bitmap to
serve a fetch request, it can erroneously die() if some of
the UNINTERESTING objects are not present. Upload-pack
throws away HAVE lines from the client for objects we do not
have, but we may have a tip object without all of its
ancestors (e.g., if the tip is no longer reachable and was
new enough to survive a `git prune`, but some of its
reachable objects did get pruned).
In the non-bitmap case, we do a revision walk with the HAVE
objects marked as UNINTERESTING. The revision walker
explicitly ignores errors in accessing UNINTERESTING commits
to handle this case (and we do not bother looking at
UNINTERESTING trees or blobs at all).
When we have bitmaps, however, the process is quite
different. The bitmap index for a pack-objects run is
calculated in two separate steps:
First, we perform an extensive walk from all the HAVEs to
find the full set of objects reachable from them. This walk
is usually optimized away because we are expected to hit an
object with a bitmap during the traversal, which allows us
to terminate early.
Secondly, we perform an extensive walk from all the WANTs,
which usually also terminates early because we hit a commit
with an existing bitmap.
Once we have the resulting bitmaps from the two walks, we
AND-NOT them together to obtain the resulting set of objects
we need to pack.
When we are walking the HAVE objects, the revision walker
does not know that we are walking it only to mark the
results as uninteresting. We strip out the UNINTERESTING flag,
because those objects _are_ interesting to us during the
first walk. We want to keep going to get a complete set of
reachable objects if we can.
We need some way to tell the revision walker that it's OK to
silently truncate the HAVE walk, just like it does for the
UNINTERESTING case. This patch introduces a new
`ignore_missing_links` flag to the `rev_info` struct, which
we set only for the HAVE walk.
It also adds tests to cover UNINTERESTING objects missing
from several positions: a missing blob, a missing tree, and
a missing parent commit. The missing blob already worked (as
we do not care about its contents at all), but the other two
cases caused us to die().
Note that there are a few cases we do not need to test:
1. We do not need to test a missing tree, with the blob
still present. Without the tree that refers to it, we
would not know that the blob is relevant to our walk.
2. We do not need to test a tip commit that is missing.
Upload-pack omits these for us (and in fact, we
complain even in the non-bitmap case if it fails to do
so).
Reported-by: Siddharth Agarwal <sid0@fb.com>
Signed-off-by: Vicent Marti <tanoku@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-28 18:00:43 +08:00
|
|
|
test_expect_success 'pack with missing blob' '
|
|
|
|
|
rm $(objpath $blob) &&
|
|
|
|
|
git pack-objects --stdout --revs <revs >/dev/null
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack with missing tree' '
|
|
|
|
|
rm $(objpath $tree) &&
|
|
|
|
|
git pack-objects --stdout --revs <revs >/dev/null
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack with missing parent' '
|
|
|
|
|
rm $(objpath $parent) &&
|
|
|
|
|
git pack-objects --stdout --revs <revs >/dev/null
|
|
|
|
|
'
|
|
|
|
|
|
2013-12-21 22:00:38 +08:00
|
|
|
test_expect_success JGIT 'we can read jgit bitmaps' '
|
2018-05-10 21:58:52 +08:00
|
|
|
git clone --bare . compat-jgit.git &&
|
2013-12-21 22:00:38 +08:00
|
|
|
(
|
2018-05-10 21:58:52 +08:00
|
|
|
cd compat-jgit.git &&
|
2019-03-15 14:22:44 +08:00
|
|
|
rm -f objects/pack/*.bitmap &&
|
2013-12-21 22:00:38 +08:00
|
|
|
jgit gc &&
|
|
|
|
|
git rev-list --test-bitmap HEAD
|
|
|
|
|
)
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success JGIT 'jgit can read our bitmaps' '
|
2018-05-10 21:58:52 +08:00
|
|
|
git clone --bare . compat-us.git &&
|
2013-12-21 22:00:38 +08:00
|
|
|
(
|
2018-05-10 21:58:52 +08:00
|
|
|
cd compat-us.git &&
|
2013-12-21 22:00:38 +08:00
|
|
|
git repack -adb &&
|
|
|
|
|
# jgit gc will barf if it does not like our bitmaps
|
|
|
|
|
jgit gc
|
|
|
|
|
)
|
|
|
|
|
'
|
|
|
|
|
|
2014-10-17 09:11:43 +08:00
|
|
|
test_expect_success 'splitting packs does not generate bogus bitmaps' '
|
2018-03-24 15:44:42 +08:00
|
|
|
test-tool genrandom foo $((1024 * 1024)) >rand &&
|
2014-10-17 09:11:43 +08:00
|
|
|
git add rand &&
|
|
|
|
|
git commit -m "commit with big file" &&
|
|
|
|
|
git -c pack.packSizeLimit=500k repack -adb &&
|
|
|
|
|
git init --bare no-bitmaps.git &&
|
|
|
|
|
git -C no-bitmaps.git fetch .. HEAD
|
|
|
|
|
'
|
|
|
|
|
|
pack-objects: disable pack reuse for object-selection options
If certain options like --honor-pack-keep, --local, or
--incremental are used with pack-objects, then we need to
feed each potential object to want_object_in_pack() to see
if it should be filtered out. But when the bitmap
reuse_packfile optimization is in effect, we do not call
that function at all, and in fact skip adding the objects to
the to_pack list entirely. This means we have a bug: for
certain requests we will silently ignore those options and
include objects in that pack that should not be there.
The problem has been present since the inception of the
pack-reuse code in 6b8fda2db (pack-objects: use bitmaps when
packing objects, 2013-12-21), but it was unlikely to come up
in practice. These options are generally used for on-disk
packing, not transfer packs (which go to stdout), but we've
never allowed pack reuse for non-stdout packs (until
645c432d6, we did not even use bitmaps, which the reuse
optimization relies on; after that, we explicitly turned it
off when not packing to stdout).
We can fix this by just disabling the reuse_packfile
optimization when the options are in use. In theory we could
teach the pack-reuse code to satisfy these checks, but it's
not worth the complexity. The purpose of the optimization is
to keep the amount of per-object work we do to a minimum.
But these options inherently require us to search for other
copies of each object, drowning out any benefit of the
pack-reuse optimization. But note that the optimizations
from 56dfeb626 (pack-objects: compute local/ignore_pack_keep
early, 2016-07-29) happen before pack-reuse, meaning that
specifying "--honor-pack-keep" in a repository with no .keep
files can still follow the fast path.
There are tests in t5310 that check these options with
bitmaps and --stdout, but they didn't catch the bug, and
it's hard to adapt them to do so.
One problem is that they don't use --delta-base-offset;
without that option, we always disable the reuse
optimization entirely. It would be fine to add it in (it
actually makes the test more realistic), but that still
isn't quite enough.
The other problem is that the reuse code is very picky; it
only kicks in when it can reuse most of a pack, starting
from the first byte. So we'd have to start from a fully
repacked and bitmapped state to trigger it. But the tests
for these options use a much more subtle state; they want to
be sure that the want_object_in_pack() code is allowing some
objects but not others. Doing a full repack runs counter to
that.
So this patch adds new tests at the end of the script which
create the fully-packed state and make sure that each option
is not fooled by reusable pack.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-09 10:54:13 +08:00
|
|
|
test_expect_success 'set up reusable pack' '
|
|
|
|
|
rm -f .git/objects/pack/*.keep &&
|
|
|
|
|
git repack -adb &&
|
|
|
|
|
reusable_pack () {
|
|
|
|
|
git for-each-ref --format="%(objectname)" |
|
|
|
|
|
git pack-objects --delta-base-offset --revs --stdout "$@"
|
|
|
|
|
}
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack reuse respects --honor-pack-keep' '
|
|
|
|
|
test_when_finished "rm -f .git/objects/pack/*.keep" &&
|
2017-05-09 10:59:46 +08:00
|
|
|
for i in .git/objects/pack/*.pack
|
|
|
|
|
do
|
pack-objects: disable pack reuse for object-selection options
If certain options like --honor-pack-keep, --local, or
--incremental are used with pack-objects, then we need to
feed each potential object to want_object_in_pack() to see
if it should be filtered out. But when the bitmap
reuse_packfile optimization is in effect, we do not call
that function at all, and in fact skip adding the objects to
the to_pack list entirely. This means we have a bug: for
certain requests we will silently ignore those options and
include objects in that pack that should not be there.
The problem has been present since the inception of the
pack-reuse code in 6b8fda2db (pack-objects: use bitmaps when
packing objects, 2013-12-21), but it was unlikely to come up
in practice. These options are generally used for on-disk
packing, not transfer packs (which go to stdout), but we've
never allowed pack reuse for non-stdout packs (until
645c432d6, we did not even use bitmaps, which the reuse
optimization relies on; after that, we explicitly turned it
off when not packing to stdout).
We can fix this by just disabling the reuse_packfile
optimization when the options are in use. In theory we could
teach the pack-reuse code to satisfy these checks, but it's
not worth the complexity. The purpose of the optimization is
to keep the amount of per-object work we do to a minimum.
But these options inherently require us to search for other
copies of each object, drowning out any benefit of the
pack-reuse optimization. But note that the optimizations
from 56dfeb626 (pack-objects: compute local/ignore_pack_keep
early, 2016-07-29) happen before pack-reuse, meaning that
specifying "--honor-pack-keep" in a repository with no .keep
files can still follow the fast path.
There are tests in t5310 that check these options with
bitmaps and --stdout, but they didn't catch the bug, and
it's hard to adapt them to do so.
One problem is that they don't use --delta-base-offset;
without that option, we always disable the reuse
optimization entirely. It would be fine to add it in (it
actually makes the test more realistic), but that still
isn't quite enough.
The other problem is that the reuse code is very picky; it
only kicks in when it can reuse most of a pack, starting
from the first byte. So we'd have to start from a fully
repacked and bitmapped state to trigger it. But the tests
for these options use a much more subtle state; they want to
be sure that the want_object_in_pack() code is allowing some
objects but not others. Doing a full repack runs counter to
that.
So this patch adds new tests at the end of the script which
create the fully-packed state and make sure that each option
is not fooled by reusable pack.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-09 10:54:13 +08:00
|
|
|
>${i%.pack}.keep
|
|
|
|
|
done &&
|
|
|
|
|
reusable_pack --honor-pack-keep >empty.pack &&
|
|
|
|
|
git index-pack empty.pack &&
|
|
|
|
|
git show-index <empty.idx >actual &&
|
2018-07-28 01:48:11 +08:00
|
|
|
test_must_be_empty actual
|
pack-objects: disable pack reuse for object-selection options
If certain options like --honor-pack-keep, --local, or
--incremental are used with pack-objects, then we need to
feed each potential object to want_object_in_pack() to see
if it should be filtered out. But when the bitmap
reuse_packfile optimization is in effect, we do not call
that function at all, and in fact skip adding the objects to
the to_pack list entirely. This means we have a bug: for
certain requests we will silently ignore those options and
include objects in that pack that should not be there.
The problem has been present since the inception of the
pack-reuse code in 6b8fda2db (pack-objects: use bitmaps when
packing objects, 2013-12-21), but it was unlikely to come up
in practice. These options are generally used for on-disk
packing, not transfer packs (which go to stdout), but we've
never allowed pack reuse for non-stdout packs (until
645c432d6, we did not even use bitmaps, which the reuse
optimization relies on; after that, we explicitly turned it
off when not packing to stdout).
We can fix this by just disabling the reuse_packfile
optimization when the options are in use. In theory we could
teach the pack-reuse code to satisfy these checks, but it's
not worth the complexity. The purpose of the optimization is
to keep the amount of per-object work we do to a minimum.
But these options inherently require us to search for other
copies of each object, drowning out any benefit of the
pack-reuse optimization. But note that the optimizations
from 56dfeb626 (pack-objects: compute local/ignore_pack_keep
early, 2016-07-29) happen before pack-reuse, meaning that
specifying "--honor-pack-keep" in a repository with no .keep
files can still follow the fast path.
There are tests in t5310 that check these options with
bitmaps and --stdout, but they didn't catch the bug, and
it's hard to adapt them to do so.
One problem is that they don't use --delta-base-offset;
without that option, we always disable the reuse
optimization entirely. It would be fine to add it in (it
actually makes the test more realistic), but that still
isn't quite enough.
The other problem is that the reuse code is very picky; it
only kicks in when it can reuse most of a pack, starting
from the first byte. So we'd have to start from a fully
repacked and bitmapped state to trigger it. But the tests
for these options use a much more subtle state; they want to
be sure that the want_object_in_pack() code is allowing some
objects but not others. Doing a full repack runs counter to
that.
So this patch adds new tests at the end of the script which
create the fully-packed state and make sure that each option
is not fooled by reusable pack.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-09 10:54:13 +08:00
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack reuse respects --local' '
|
|
|
|
|
mv .git/objects/pack/* alt.git/objects/pack/ &&
|
|
|
|
|
test_when_finished "mv alt.git/objects/pack/* .git/objects/pack/" &&
|
|
|
|
|
reusable_pack --local >empty.pack &&
|
|
|
|
|
git index-pack empty.pack &&
|
|
|
|
|
git show-index <empty.idx >actual &&
|
2018-07-28 01:48:11 +08:00
|
|
|
test_must_be_empty actual
|
pack-objects: disable pack reuse for object-selection options
If certain options like --honor-pack-keep, --local, or
--incremental are used with pack-objects, then we need to
feed each potential object to want_object_in_pack() to see
if it should be filtered out. But when the bitmap
reuse_packfile optimization is in effect, we do not call
that function at all, and in fact skip adding the objects to
the to_pack list entirely. This means we have a bug: for
certain requests we will silently ignore those options and
include objects in that pack that should not be there.
The problem has been present since the inception of the
pack-reuse code in 6b8fda2db (pack-objects: use bitmaps when
packing objects, 2013-12-21), but it was unlikely to come up
in practice. These options are generally used for on-disk
packing, not transfer packs (which go to stdout), but we've
never allowed pack reuse for non-stdout packs (until
645c432d6, we did not even use bitmaps, which the reuse
optimization relies on; after that, we explicitly turned it
off when not packing to stdout).
We can fix this by just disabling the reuse_packfile
optimization when the options are in use. In theory we could
teach the pack-reuse code to satisfy these checks, but it's
not worth the complexity. The purpose of the optimization is
to keep the amount of per-object work we do to a minimum.
But these options inherently require us to search for other
copies of each object, drowning out any benefit of the
pack-reuse optimization. But note that the optimizations
from 56dfeb626 (pack-objects: compute local/ignore_pack_keep
early, 2016-07-29) happen before pack-reuse, meaning that
specifying "--honor-pack-keep" in a repository with no .keep
files can still follow the fast path.
There are tests in t5310 that check these options with
bitmaps and --stdout, but they didn't catch the bug, and
it's hard to adapt them to do so.
One problem is that they don't use --delta-base-offset;
without that option, we always disable the reuse
optimization entirely. It would be fine to add it in (it
actually makes the test more realistic), but that still
isn't quite enough.
The other problem is that the reuse code is very picky; it
only kicks in when it can reuse most of a pack, starting
from the first byte. So we'd have to start from a fully
repacked and bitmapped state to trigger it. But the tests
for these options use a much more subtle state; they want to
be sure that the want_object_in_pack() code is allowing some
objects but not others. Doing a full repack runs counter to
that.
So this patch adds new tests at the end of the script which
create the fully-packed state and make sure that each option
is not fooled by reusable pack.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-09 10:54:13 +08:00
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack reuse respects --incremental' '
|
|
|
|
|
reusable_pack --incremental >empty.pack &&
|
|
|
|
|
git index-pack empty.pack &&
|
|
|
|
|
git show-index <empty.idx >actual &&
|
2018-07-28 01:48:11 +08:00
|
|
|
test_must_be_empty actual
|
pack-objects: disable pack reuse for object-selection options
If certain options like --honor-pack-keep, --local, or
--incremental are used with pack-objects, then we need to
feed each potential object to want_object_in_pack() to see
if it should be filtered out. But when the bitmap
reuse_packfile optimization is in effect, we do not call
that function at all, and in fact skip adding the objects to
the to_pack list entirely. This means we have a bug: for
certain requests we will silently ignore those options and
include objects in that pack that should not be there.
The problem has been present since the inception of the
pack-reuse code in 6b8fda2db (pack-objects: use bitmaps when
packing objects, 2013-12-21), but it was unlikely to come up
in practice. These options are generally used for on-disk
packing, not transfer packs (which go to stdout), but we've
never allowed pack reuse for non-stdout packs (until
645c432d6, we did not even use bitmaps, which the reuse
optimization relies on; after that, we explicitly turned it
off when not packing to stdout).
We can fix this by just disabling the reuse_packfile
optimization when the options are in use. In theory we could
teach the pack-reuse code to satisfy these checks, but it's
not worth the complexity. The purpose of the optimization is
to keep the amount of per-object work we do to a minimum.
But these options inherently require us to search for other
copies of each object, drowning out any benefit of the
pack-reuse optimization. But note that the optimizations
from 56dfeb626 (pack-objects: compute local/ignore_pack_keep
early, 2016-07-29) happen before pack-reuse, meaning that
specifying "--honor-pack-keep" in a repository with no .keep
files can still follow the fast path.
There are tests in t5310 that check these options with
bitmaps and --stdout, but they didn't catch the bug, and
it's hard to adapt them to do so.
One problem is that they don't use --delta-base-offset;
without that option, we always disable the reuse
optimization entirely. It would be fine to add it in (it
actually makes the test more realistic), but that still
isn't quite enough.
The other problem is that the reuse code is very picky; it
only kicks in when it can reuse most of a pack, starting
from the first byte. So we'd have to start from a fully
repacked and bitmapped state to trigger it. But the tests
for these options use a much more subtle state; they want to
be sure that the want_object_in_pack() code is allowing some
objects but not others. Doing a full repack runs counter to
that.
So this patch adds new tests at the end of the script which
create the fully-packed state and make sure that each option
is not fooled by reusable pack.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-09 10:54:13 +08:00
|
|
|
'
|
2018-06-15 11:31:13 +08:00
|
|
|
|
|
|
|
|
test_expect_success 'truncated bitmap fails gracefully' '
|
|
|
|
|
git repack -ad &&
|
|
|
|
|
git rev-list --use-bitmap-index --count --all >expect &&
|
|
|
|
|
bitmap=$(ls .git/objects/pack/*.bitmap) &&
|
|
|
|
|
test_when_finished "rm -f $bitmap" &&
|
2018-08-24 23:20:11 +08:00
|
|
|
test_copy_bytes 512 <$bitmap >$bitmap.tmp &&
|
2018-06-15 11:31:13 +08:00
|
|
|
mv -f $bitmap.tmp $bitmap &&
|
|
|
|
|
git rev-list --use-bitmap-index --count --all >actual 2>stderr &&
|
|
|
|
|
test_cmp expect actual &&
|
|
|
|
|
test_i18ngrep corrupt stderr
|
|
|
|
|
'
|
|
|
|
|
|
2018-09-01 15:48:13 +08:00
|
|
|
# have_delta <obj> <expected_base>
|
|
|
|
|
#
|
|
|
|
|
# Note that because this relies on cat-file, it might find _any_ copy of an
|
|
|
|
|
# object in the repository. The caller is responsible for making sure
|
|
|
|
|
# there's only one (e.g., via "repack -ad", or having just fetched a copy).
|
|
|
|
|
have_delta () {
|
|
|
|
|
echo $2 >expect &&
|
|
|
|
|
echo $1 | git cat-file --batch-check="%(deltabase)" >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
# Create a state of history with these properties:
|
|
|
|
|
#
|
|
|
|
|
# - refs that allow a client to fetch some new history, while sharing some old
|
|
|
|
|
# history with the server; we use branches delta-reuse-old and
|
|
|
|
|
# delta-reuse-new here
|
|
|
|
|
#
|
|
|
|
|
# - the new history contains an object that is stored on the server as a delta
|
|
|
|
|
# against a base that is in the old history
|
|
|
|
|
#
|
|
|
|
|
# - the base object is not immediately reachable from the tip of the old
|
|
|
|
|
# history; finding it would involve digging down through history we know the
|
|
|
|
|
# other side has
|
|
|
|
|
#
|
|
|
|
|
# This should result in a state where fetching from old->new would not
|
|
|
|
|
# traditionally reuse the on-disk delta (because we'd have to dig to realize
|
|
|
|
|
# that the client has it), but we will do so if bitmaps can tell us cheaply
|
|
|
|
|
# that the other side has it.
|
|
|
|
|
test_expect_success 'set up thin delta-reuse parent' '
|
|
|
|
|
# This first commit contains the buried base object.
|
|
|
|
|
test-tool genrandom delta 16384 >file &&
|
|
|
|
|
git add file &&
|
|
|
|
|
git commit -m "delta base" &&
|
|
|
|
|
base=$(git rev-parse --verify HEAD:file) &&
|
|
|
|
|
|
|
|
|
|
# These intermediate commits bury the base back in history.
|
|
|
|
|
# This becomes the "old" state.
|
|
|
|
|
for i in 1 2 3 4 5
|
|
|
|
|
do
|
|
|
|
|
echo $i >file &&
|
|
|
|
|
git commit -am "intermediate $i" || return 1
|
|
|
|
|
done &&
|
|
|
|
|
git branch delta-reuse-old &&
|
|
|
|
|
|
|
|
|
|
# And now our new history has a delta against the buried base. Note
|
|
|
|
|
# that this must be smaller than the original file, since pack-objects
|
|
|
|
|
# prefers to create deltas from smaller objects to larger.
|
|
|
|
|
test-tool genrandom delta 16300 >file &&
|
|
|
|
|
git commit -am "delta result" &&
|
|
|
|
|
delta=$(git rev-parse --verify HEAD:file) &&
|
|
|
|
|
git branch delta-reuse-new &&
|
|
|
|
|
|
|
|
|
|
# Repack with bitmaps and double check that we have the expected delta
|
|
|
|
|
# relationship.
|
|
|
|
|
git repack -adb &&
|
|
|
|
|
have_delta $delta $base
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
# Now we can sanity-check the non-bitmap behavior (that the server is not able
|
|
|
|
|
# to reuse the delta). This isn't strictly something we care about, so this
|
|
|
|
|
# test could be scrapped in the future. But it makes sure that the next test is
|
|
|
|
|
# actually triggering the feature we want.
|
|
|
|
|
#
|
|
|
|
|
# Note that our tools for working with on-the-wire "thin" packs are limited. So
|
|
|
|
|
# we actually perform the fetch, retain the resulting pack, and inspect the
|
|
|
|
|
# result.
|
|
|
|
|
test_expect_success 'fetch without bitmaps ignores delta against old base' '
|
|
|
|
|
test_config pack.usebitmaps false &&
|
|
|
|
|
test_when_finished "rm -rf client.git" &&
|
|
|
|
|
git init --bare client.git &&
|
|
|
|
|
(
|
|
|
|
|
cd client.git &&
|
|
|
|
|
git config transfer.unpackLimit 1 &&
|
|
|
|
|
git fetch .. delta-reuse-old:delta-reuse-old &&
|
|
|
|
|
git fetch .. delta-reuse-new:delta-reuse-new &&
|
|
|
|
|
have_delta $delta $ZERO_OID
|
|
|
|
|
)
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
# And do the same for the bitmap case, where we do expect to find the delta.
|
|
|
|
|
test_expect_success 'fetch with bitmaps can reuse old base' '
|
|
|
|
|
test_config pack.usebitmaps true &&
|
|
|
|
|
test_when_finished "rm -rf client.git" &&
|
|
|
|
|
git init --bare client.git &&
|
|
|
|
|
(
|
|
|
|
|
cd client.git &&
|
|
|
|
|
git config transfer.unpackLimit 1 &&
|
|
|
|
|
git fetch .. delta-reuse-old:delta-reuse-old &&
|
|
|
|
|
git fetch .. delta-reuse-new:delta-reuse-new &&
|
|
|
|
|
have_delta $delta $base
|
|
|
|
|
)
|
|
|
|
|
'
|
|
|
|
|
|
2013-12-21 22:00:38 +08:00
|
|
|
test_done
|
