2013-12-21 18:00:38 +04: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 14:00:43 +04: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 18:01:10 +03:00
|
|
|
# show objects present in pack ($1 should be associated *.idx)
|
|
|
|
|
list_packed_objects () {
|
|
|
|
|
git show-index <"$1" | cut -d' ' -f2
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
# 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 18:00:38 +04:00
|
|
|
test_expect_success 'setup repo with moderate-sized history' '
|
2017-05-09 05:59:46 +03:00
|
|
|
for i in $(test_seq 1 10)
|
|
|
|
|
do
|
2013-12-21 18:00:38 +04:00
|
|
|
test_commit $i
|
|
|
|
|
done &&
|
|
|
|
|
git checkout -b other HEAD~5 &&
|
2017-05-09 05:59:46 +03:00
|
|
|
for i in $(test_seq 1 10)
|
|
|
|
|
do
|
2013-12-21 18:00:38 +04:00
|
|
|
test_commit side-$i
|
|
|
|
|
done &&
|
|
|
|
|
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 18:01:10 +03:00
|
|
|
bitmaptip=$(git rev-parse master) &&
|
2013-12-21 18:00:38 +04:00
|
|
|
blob=$(echo tagged-blob | git hash-object -w --stdin) &&
|
|
|
|
|
git tag tagged-blob $blob &&
|
2014-06-11 00:20:30 +04:00
|
|
|
git config repack.writebitmaps true &&
|
pack-bitmap: implement optional name_hash cache
When we use pack bitmaps rather than walking the object
graph, we end up with the list of objects to include in the
packfile, but we do not know the path at which any tree or
blob objects would be found.
In a recently packed repository, this is fine. A fetch would
use the paths only as a heuristic in the delta compression
phase, and a fully packed repository should not need to do
much delta compression.
As time passes, though, we may acquire more objects on top
of our large bitmapped pack. If clients fetch frequently,
then they never even look at the bitmapped history, and all
works as usual. However, a client who has not fetched since
the last bitmap repack will have "have" tips in the
bitmapped history, but "want" newer objects.
The bitmaps themselves degrade gracefully in this
circumstance. We manually walk the more recent bits of
history, and then use bitmaps when we hit them.
But we would also like to perform delta compression between
the newer objects and the bitmapped objects (both to delta
against what we know the user already has, but also between
"new" and "old" objects that the user is fetching). The lack
of pathnames makes our delta heuristics much less effective.
This patch adds an optional cache of the 32-bit name_hash
values to the end of the bitmap file. If present, a reader
can use it to match bitmapped and non-bitmapped names during
delta compression.
Here are perf results for p5310:
Test origin/master HEAD^ HEAD
-------------------------------------------------------------------------------------------------
5310.2: repack to disk 36.81(37.82+1.43) 47.70(48.74+1.41) +29.6% 47.75(48.70+1.51) +29.7%
5310.3: simulated clone 30.78(29.70+2.14) 1.08(0.97+0.10) -96.5% 1.07(0.94+0.12) -96.5%
5310.4: simulated fetch 3.16(6.10+0.08) 3.54(10.65+0.06) +12.0% 1.70(3.07+0.06) -46.2%
5310.6: partial bitmap 36.76(43.19+1.81) 6.71(11.25+0.76) -81.7% 4.08(6.26+0.46) -88.9%
You can see that the time spent on an incremental fetch goes
down, as our delta heuristics are able to do their work.
And we save time on the partial bitmap clone for the same
reason.
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>
2013-12-21 18:00:45 +04:00
|
|
|
git config pack.writebitmaphashcache true
|
2013-12-21 18:00:38 +04: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 10:07:34 +03: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 18:00:38 +04: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-01 21:42:17 +03: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
|
|
|
|
|
'
|
|
|
|
|
|
2013-12-21 18:00:38 +04:00
|
|
|
test_expect_success "enumerate --objects ($state)" '
|
|
|
|
|
git rev-list --objects --use-bitmap-index HEAD >tmp &&
|
|
|
|
|
cut -d" " -f1 <tmp >tmp2 &&
|
|
|
|
|
sort <tmp2 >actual &&
|
|
|
|
|
git rev-list --objects HEAD >tmp &&
|
|
|
|
|
cut -d" " -f1 <tmp >tmp2 &&
|
|
|
|
|
sort <tmp2 >expect &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
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
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'setup further non-bitmapped commits' '
|
2017-05-09 05:59:46 +03:00
|
|
|
for i in $(test_seq 1 10)
|
|
|
|
|
do
|
2013-12-21 18:00:38 +04:00
|
|
|
test_commit further-$i
|
|
|
|
|
done
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
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 01:45:42 +03:00
|
|
|
test_expect_success 'incremental repack fails when bitmaps are requested' '
|
2013-12-21 18:00:38 +04:00
|
|
|
test_commit more-1 &&
|
2016-12-29 01:45:42 +03:00
|
|
|
test_must_fail git repack -d 2>err &&
|
|
|
|
|
test_i18ngrep "Incremental repacks are incompatible with bitmap" err
|
2013-12-21 18:00:38 +04: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 18:01:10 +03: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/ &&
|
|
|
|
|
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 18:01:44 +03: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) &&
|
|
|
|
|
list_packed_objects <packa-$packasha1.idx >packa.objects &&
|
|
|
|
|
list_packed_objects <packb-$packbsha1.idx >packb.objects &&
|
|
|
|
|
test_cmp packa.objects packb.objects
|
|
|
|
|
'
|
|
|
|
|
|
2013-12-21 18:00:38 +04: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 14:00:43 +04: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 18:01:10 +03: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 14:00:43 +04: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 18:00:38 +04:00
|
|
|
test_expect_success JGIT 'we can read jgit bitmaps' '
|
|
|
|
|
git clone . compat-jgit &&
|
|
|
|
|
(
|
|
|
|
|
cd compat-jgit &&
|
|
|
|
|
rm -f .git/objects/pack/*.bitmap &&
|
|
|
|
|
jgit gc &&
|
|
|
|
|
git rev-list --test-bitmap HEAD
|
|
|
|
|
)
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success JGIT 'jgit can read our bitmaps' '
|
|
|
|
|
git clone . compat-us &&
|
|
|
|
|
(
|
|
|
|
|
cd compat-us &&
|
|
|
|
|
git repack -adb &&
|
|
|
|
|
# jgit gc will barf if it does not like our bitmaps
|
|
|
|
|
jgit gc
|
|
|
|
|
)
|
|
|
|
|
'
|
|
|
|
|
|
2014-10-17 05:11:43 +04:00
|
|
|
test_expect_success 'splitting packs does not generate bogus bitmaps' '
|
|
|
|
|
test-genrandom foo $((1024 * 1024)) >rand &&
|
|
|
|
|
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 05:54:13 +03: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 05:59:46 +03: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 05:54:13 +03:00
|
|
|
>${i%.pack}.keep
|
|
|
|
|
done &&
|
|
|
|
|
reusable_pack --honor-pack-keep >empty.pack &&
|
|
|
|
|
git index-pack empty.pack &&
|
|
|
|
|
>expect &&
|
|
|
|
|
git show-index <empty.idx >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
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 &&
|
|
|
|
|
>expect &&
|
|
|
|
|
git show-index <empty.idx >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'pack reuse respects --incremental' '
|
|
|
|
|
reusable_pack --incremental >empty.pack &&
|
|
|
|
|
git index-pack empty.pack &&
|
|
|
|
|
>expect &&
|
|
|
|
|
git show-index <empty.idx >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
'
|
2013-12-21 18:00:38 +04:00
|
|
|
test_done
|
