From cf79bd9f4c34c635a5a7eeff2699a24744c5a129 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 25 Oct 2017 11:53:20 +0200 Subject: t1409: check that `packed-refs` is not rewritten unnecessarily There is no need to rewrite the `packed-refs` file except for the case that we are deleting a reference that has a packed version. Verify that `packed-refs` is not rewritten when it shouldn't be. In fact, two of these tests fail: * A new (empty) `packed-refs` file is created when deleting any loose reference and no `packed-refs` file previously existed. * The `packed-refs` file is rewritten unnecessarily when deleting a loose reference that has no packed counterpart. Both problems will be fixed in the next commit. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- t/t1409-avoid-packing-refs.sh | 118 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100755 t/t1409-avoid-packing-refs.sh (limited to 't') diff --git a/t/t1409-avoid-packing-refs.sh b/t/t1409-avoid-packing-refs.sh new file mode 100755 index 0000000000..a2397c7b71 --- /dev/null +++ b/t/t1409-avoid-packing-refs.sh @@ -0,0 +1,118 @@ +#!/bin/sh + +test_description='avoid rewriting packed-refs unnecessarily' + +. ./test-lib.sh + +# Add an identifying mark to the packed-refs file header line. This +# shouldn't upset readers, and it should be omitted if the file is +# ever rewritten. +mark_packed_refs () { + sed -e "s/^\(#.*\)/\1 t1409 /" <.git/packed-refs >.git/packed-refs.new && + mv .git/packed-refs.new .git/packed-refs +} + +# Verify that the packed-refs file is still marked. +check_packed_refs_marked () { + grep -q '^#.* t1409 ' .git/packed-refs +} + +test_expect_success 'setup' ' + git commit --allow-empty -m "Commit A" && + A=$(git rev-parse HEAD) && + git commit --allow-empty -m "Commit B" && + B=$(git rev-parse HEAD) && + git commit --allow-empty -m "Commit C" && + C=$(git rev-parse HEAD) +' + +test_expect_failure 'do not create packed-refs file gratuitously' ' + test_must_fail test -f .git/packed-refs && + git update-ref refs/heads/foo $A && + test_must_fail test -f .git/packed-refs && + git update-ref refs/heads/foo $B && + test_must_fail test -f .git/packed-refs && + git update-ref refs/heads/foo $C $B && + test_must_fail test -f .git/packed-refs && + git update-ref -d refs/heads/foo && + test_must_fail test -f .git/packed-refs +' + +test_expect_success 'check that marking the packed-refs file works' ' + git for-each-ref >expected && + git pack-refs --all && + mark_packed_refs && + check_packed_refs_marked && + git for-each-ref >actual && + test_cmp expected actual && + git pack-refs --all && + test_must_fail check_packed_refs_marked && + git for-each-ref >actual2 && + test_cmp expected actual2 +' + +test_expect_success 'leave packed-refs untouched on update of packed' ' + git update-ref refs/heads/packed-update $A && + git pack-refs --all && + mark_packed_refs && + git update-ref refs/heads/packed-update $B && + check_packed_refs_marked +' + +test_expect_success 'leave packed-refs untouched on checked update of packed' ' + git update-ref refs/heads/packed-checked-update $A && + git pack-refs --all && + mark_packed_refs && + git update-ref refs/heads/packed-checked-update $B $A && + check_packed_refs_marked +' + +test_expect_success 'leave packed-refs untouched on verify of packed' ' + git update-ref refs/heads/packed-verify $A && + git pack-refs --all && + mark_packed_refs && + echo "verify refs/heads/packed-verify $A" | git update-ref --stdin && + check_packed_refs_marked +' + +test_expect_success 'touch packed-refs on delete of packed' ' + git update-ref refs/heads/packed-delete $A && + git pack-refs --all && + mark_packed_refs && + git update-ref -d refs/heads/packed-delete && + test_must_fail check_packed_refs_marked +' + +test_expect_success 'leave packed-refs untouched on update of loose' ' + git pack-refs --all && + git update-ref refs/heads/loose-update $A && + mark_packed_refs && + git update-ref refs/heads/loose-update $B && + check_packed_refs_marked +' + +test_expect_success 'leave packed-refs untouched on checked update of loose' ' + git pack-refs --all && + git update-ref refs/heads/loose-checked-update $A && + mark_packed_refs && + git update-ref refs/heads/loose-checked-update $B $A && + check_packed_refs_marked +' + +test_expect_success 'leave packed-refs untouched on verify of loose' ' + git pack-refs --all && + git update-ref refs/heads/loose-verify $A && + mark_packed_refs && + echo "verify refs/heads/loose-verify $A" | git update-ref --stdin && + check_packed_refs_marked +' + +test_expect_failure 'leave packed-refs untouched on delete of loose' ' + git pack-refs --all && + git update-ref refs/heads/loose-delete $A && + mark_packed_refs && + git update-ref -d refs/heads/loose-delete && + check_packed_refs_marked +' + +test_done -- cgit v1.2.3 From 7c6bd25c7d65e777b5bb26ad5c8248fb3607e6d5 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 28 Oct 2017 11:16:02 +0200 Subject: files-backend: don't rewrite the `packed-refs` file unnecessarily Even when we are deleting references, we needn't overwrite the `packed-refs` file if the references that we are deleting only exist as loose references. Implement this optimization as follows: * Add a function `is_packed_transaction_needed()`, which checks whether a given packed-refs transaction actually needs to be carried out (i.e., it returns false if the transaction obviously wouldn't have any effect). This function must be called while holding the `packed-refs` lock to avoid races. * Change `files_transaction_prepare()` to check whether the packed-refs transaction is actually needed. If not, squelch it, but continue holding the `packed-refs` lock until the end of the transaction to avoid races. This fixes a mild regression caused by dc39e09942 (files_ref_store: use a transaction to update packed refs, 2017-09-08). Before that commit, unnecessary rewrites of `packed-refs` were suppressed by `repack_without_refs()`. But the transaction-based writing introduced by that commit didn't perform that optimization. Note that the pre-dc39e09942 code still had to *read* the whole `packed-refs` file to determine that the rewrite could be skipped, so the performance for the cases that the write could be elided was `O(N)` in the number of packed references both before and after dc39e09942. But after that commit the constant factor increased. This commit reimplements the optimization of eliding unnecessary `packed-refs` rewrites. That, plus the fact that since cfa2e29c34 (packed_ref_store: get rid of the `ref_cache` entirely, 2017-03-17) we don't necessarily have to read the whole `packed-refs` file at all, means that deletes of one or a few loose references can now be done with `O(n lg N)` effort, where `n` is the number of loose references being deleted and `N` is the total number of packed references. This commit fixes two tests in t1409. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- t/t1409-avoid-packing-refs.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t1409-avoid-packing-refs.sh b/t/t1409-avoid-packing-refs.sh index a2397c7b71..e5cb8a252d 100755 --- a/t/t1409-avoid-packing-refs.sh +++ b/t/t1409-avoid-packing-refs.sh @@ -26,7 +26,7 @@ test_expect_success 'setup' ' C=$(git rev-parse HEAD) ' -test_expect_failure 'do not create packed-refs file gratuitously' ' +test_expect_success 'do not create packed-refs file gratuitously' ' test_must_fail test -f .git/packed-refs && git update-ref refs/heads/foo $A && test_must_fail test -f .git/packed-refs && @@ -107,7 +107,7 @@ test_expect_success 'leave packed-refs untouched on verify of loose' ' check_packed_refs_marked ' -test_expect_failure 'leave packed-refs untouched on delete of loose' ' +test_expect_success 'leave packed-refs untouched on delete of loose' ' git pack-refs --all && git update-ref refs/heads/loose-delete $A && mark_packed_refs && -- cgit v1.2.3