On Wed, Feb 16, 2022 at 05:40:19PM -0800, Elijah Newren wrote: > On Fri, Feb 11, 2022 at 12:25 AM Patrick Steinhardt <ps@xxxxxx> wrote: > > > > When fetching with the `--prune` flag we will delete any local > > references matching the fetch refspec which have disappeared on the > > remote. This step is not currently covered by the `--atomic` flag: we > > delete branches even though updating of local references has failed, > > which means that the fetch is not an all-or-nothing operation. > > > > Fix this bug by passing in the global transaction into `prune_refs()`: > > if one is given, then we'll only queue up deletions and not commit them > > right away. > > > > This change also improves performance when pruning many branches in a > > repository with a big packed-refs file: every references is pruned in > > its own transaction, which means that we potentially have to rewrite > > the packed-refs files for every single reference we're about to prune. > > > > The following benchmark demonstrates this: it performs a pruning fetch > > from a repository with a single reference into a repository with 100k > > references, which causes us to prune all but one reference. This is of > > course a very artificial setup, but serves to demonstrate the impact of > > only having to write the packed-refs file once: > > > > Benchmark 1: git fetch --prune --atomic +refs/*:refs/* (HEAD~) > > Time (mean ± σ): 2.366 s ± 0.021 s [User: 0.858 s, System: 1.508 s] > > Range (min … max): 2.328 s … 2.407 s 10 runs > > > > Benchmark 2: git fetch --prune --atomic +refs/*:refs/* (HEAD) > > Time (mean ± σ): 1.369 s ± 0.017 s [User: 0.715 s, System: 0.641 s] > > Range (min … max): 1.346 s … 1.400 s 10 runs > > > > Summary > > 'git fetch --prune --atomic +refs/*:refs/* (HEAD)' ran > > 1.73 ± 0.03 times faster than 'git fetch --prune --atomic +refs/*:refs/* (HEAD~)' > > Very nice! > > [...] > > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh > > index 93a0db3c68..afa6bf9f7d 100755 > > --- a/t/t5510-fetch.sh > > +++ b/t/t5510-fetch.sh > > @@ -349,11 +349,9 @@ test_expect_success 'fetch --atomic --prune executes a single reference transact > > cat >expected <<-EOF && > > prepared > > $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion > > - committed > > - $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion > > - prepared > > $ZERO_OID $head_oid refs/remotes/origin/new-branch > > committed > > Up to here this is just what I expected. > > > + $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion > > $ZERO_OID $head_oid refs/remotes/origin/new-branch > > But now scheduled-for-deletion and new-branch are both listed again > even with your fixes? Is this some peculiarity of the reference > transaction hook that it lists the refs again after the > prepared...committed block? (It may well be; I'm not that familiar > with this area of the code.) Yes, the reference-transaction hook is executed for each of the "prepared", "committed" and "aborted" phases and gets as input all the refs that have been queued for that phase. Patrick
Attachment:
signature.asc
Description: PGP signature
