Patchwork [5,of,5,V2] rebase: move actual rebase into a single transaction

login
register
mail settings
Submitter Durham Goode
Date March 8, 2017, 12:37 a.m.
Message ID <39116126d70a5cc2fe33.1488933457@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/18976/
State Accepted
Headers show

Comments

Durham Goode - March 8, 2017, 12:37 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1488932852 28800
#      Tue Mar 07 16:27:32 2017 -0800
# Node ID 39116126d70a5cc2fe330c880093beffa640aa64
# Parent  177c391caab3912e403e0eec469ea45a18c001f2
rebase: move actual rebase into a single transaction

Previously, rebasing would open several transaction over the course of rebasing
several commits. Opening a transaction can have notable overhead (like copying
the dirstate) which can add up when rebasing many commits.

This patch adds a single large transaction around the actual commit rebase
operation, with a catch for intervention which serializes the current state if
we need to drop back to the terminal for user intervention. Amazingly, almost
all the tests seem to pass.

On large repos with large working copies, this can speed up rebasing 7 commits
by 25%. I'd expect the percentage to be a bit larger for rebasing even more
commits.

There are minor test changes because we're rolling back the entire transaction
during unexpected exceptions instead of just stopping mid-rebase, so there's no
more backup bundle. It also leave an unknown file in the working copy, since our
clean up 'hg update' doesn't delete unknown files.
Augie Fackler - March 8, 2017, 4:27 p.m.
On Tue, Mar 07, 2017 at 04:37:37PM -0800, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1488932852 28800
> #      Tue Mar 07 16:27:32 2017 -0800
> # Node ID 39116126d70a5cc2fe330c880093beffa640aa64
> # Parent  177c391caab3912e403e0eec469ea45a18c001f2
> rebase: move actual rebase into a single transaction

I'm comfortable with these, very nice. Queued.

>
> Previously, rebasing would open several transaction over the course of rebasing
> several commits. Opening a transaction can have notable overhead (like copying
> the dirstate) which can add up when rebasing many commits.
>
> This patch adds a single large transaction around the actual commit rebase
> operation, with a catch for intervention which serializes the current state if
> we need to drop back to the terminal for user intervention. Amazingly, almost
> all the tests seem to pass.
>
> On large repos with large working copies, this can speed up rebasing 7 commits
> by 25%. I'd expect the percentage to be a bit larger for rebasing even more
> commits.
>
> There are minor test changes because we're rolling back the entire transaction
> during unexpected exceptions instead of just stopping mid-rebase, so there's no
> more backup bundle. It also leave an unknown file in the working copy, since our
> clean up 'hg update' doesn't delete unknown files.
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -343,7 +343,7 @@ class rebaseruntime(object):
>          if dest.closesbranch() and not self.keepbranchesf:
>              self.ui.status(_('reopening closed branch head %s\n') % dest)
>
> -    def _performrebase(self):
> +    def _performrebase(self, tr):
>          repo, ui, opts = self.repo, self.ui, self.opts
>          if self.keepbranchesf:
>              # insert _savebranch at the start of extrafns so if
> @@ -393,7 +393,7 @@ class rebaseruntime(object):
>                                               self.state,
>                                               self.targetancestors,
>                                               self.obsoletenotrebased)
> -                self.storestatus()
> +                self.storestatus(tr=tr)
>                  storecollapsemsg(repo, self.collapsemsg)
>                  if len(repo[None].parents()) == 2:
>                      repo.ui.debug('resuming interrupted rebase\n')
> @@ -711,7 +711,12 @@ def rebase(ui, repo, **opts):
>              if retcode is not None:
>                  return retcode
>
> -        rbsrt._performrebase()
> +        with repo.transaction('rebase') as tr:
> +            try:
> +                rbsrt._performrebase(tr)
> +            except error.InterventionRequired:
> +                tr.close()
> +                raise
>          rbsrt._finishrebase()
>      finally:
>          release(lock, wlock)
> diff --git a/tests/test-rebase-abort.t b/tests/test-rebase-abort.t
> --- a/tests/test-rebase-abort.t
> +++ b/tests/test-rebase-abort.t
> @@ -374,10 +374,11 @@ test aborting an interrupted series (iss
>    $ hg --config extensions.n=$TESTDIR/failfilemerge.py rebase -s 3 -d tip
>    rebasing 3:3a71550954f1 "b"
>    rebasing 4:e80b69427d80 "c"
> +  transaction abort!
> +  rollback completed
>    abort: ^C
>    [255]
>    $ hg rebase --abort
> -  saved backup bundle to $TESTTMP/interrupted/.hg/strip-backup/3d8812cf300d-93041a90-backup.hg (glob)
>    rebase aborted
>    $ hg log -G --template "{rev} {desc} {bookmarks}"
>    o  6 no-a
> @@ -398,7 +399,7 @@ test aborting an interrupted series (iss
>    parent: 0:df4f53cec30a
>     base
>    branch: default
> -  commit: (clean)
> +  commit: 1 unknown (clean)
>    update: 6 new changesets (update)
>    phases: 7 draft
>
> diff --git a/tests/test-rebase-conflicts.t b/tests/test-rebase-conflicts.t
> --- a/tests/test-rebase-conflicts.t
> +++ b/tests/test-rebase-conflicts.t
> @@ -225,7 +225,6 @@ Check that the right ancestors is used w
>    ignoring null merge rebase of 8
>    rebasing 9:e31216eec445 "more changes to f1"
>     future parents are 2 and -1
> -  rebase status stored
>     update to 2:4bc80088dc6b
>    resolving manifests
>     branchmerge: False, force: True, partial: False
> @@ -251,7 +250,6 @@ Check that the right ancestors is used w
>    rebased as 19c888675e13
>    rebasing 10:2f2496ddf49d "merge" (tip)
>     future parents are 11 and 7
> -  rebase status stored
>     already in target
>     merge against 10:2f2496ddf49d
>       detach base 9:e31216eec445
> @@ -269,6 +267,7 @@ Check that the right ancestors is used w
>    committing changelog
>    rebased as 2a7f09cac94c
>    rebase merging completed
> +  rebase status stored
>    update back to initial working directory parent
>    resolving manifests
>     branchmerge: False, force: False, partial: False
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -343,7 +343,7 @@  class rebaseruntime(object):
         if dest.closesbranch() and not self.keepbranchesf:
             self.ui.status(_('reopening closed branch head %s\n') % dest)
 
-    def _performrebase(self):
+    def _performrebase(self, tr):
         repo, ui, opts = self.repo, self.ui, self.opts
         if self.keepbranchesf:
             # insert _savebranch at the start of extrafns so if
@@ -393,7 +393,7 @@  class rebaseruntime(object):
                                              self.state,
                                              self.targetancestors,
                                              self.obsoletenotrebased)
-                self.storestatus()
+                self.storestatus(tr=tr)
                 storecollapsemsg(repo, self.collapsemsg)
                 if len(repo[None].parents()) == 2:
                     repo.ui.debug('resuming interrupted rebase\n')
@@ -711,7 +711,12 @@  def rebase(ui, repo, **opts):
             if retcode is not None:
                 return retcode
 
-        rbsrt._performrebase()
+        with repo.transaction('rebase') as tr:
+            try:
+                rbsrt._performrebase(tr)
+            except error.InterventionRequired:
+                tr.close()
+                raise
         rbsrt._finishrebase()
     finally:
         release(lock, wlock)
diff --git a/tests/test-rebase-abort.t b/tests/test-rebase-abort.t
--- a/tests/test-rebase-abort.t
+++ b/tests/test-rebase-abort.t
@@ -374,10 +374,11 @@  test aborting an interrupted series (iss
   $ hg --config extensions.n=$TESTDIR/failfilemerge.py rebase -s 3 -d tip
   rebasing 3:3a71550954f1 "b"
   rebasing 4:e80b69427d80 "c"
+  transaction abort!
+  rollback completed
   abort: ^C
   [255]
   $ hg rebase --abort
-  saved backup bundle to $TESTTMP/interrupted/.hg/strip-backup/3d8812cf300d-93041a90-backup.hg (glob)
   rebase aborted
   $ hg log -G --template "{rev} {desc} {bookmarks}"
   o  6 no-a
@@ -398,7 +399,7 @@  test aborting an interrupted series (iss
   parent: 0:df4f53cec30a 
    base
   branch: default
-  commit: (clean)
+  commit: 1 unknown (clean)
   update: 6 new changesets (update)
   phases: 7 draft
 
diff --git a/tests/test-rebase-conflicts.t b/tests/test-rebase-conflicts.t
--- a/tests/test-rebase-conflicts.t
+++ b/tests/test-rebase-conflicts.t
@@ -225,7 +225,6 @@  Check that the right ancestors is used w
   ignoring null merge rebase of 8
   rebasing 9:e31216eec445 "more changes to f1"
    future parents are 2 and -1
-  rebase status stored
    update to 2:4bc80088dc6b
   resolving manifests
    branchmerge: False, force: True, partial: False
@@ -251,7 +250,6 @@  Check that the right ancestors is used w
   rebased as 19c888675e13
   rebasing 10:2f2496ddf49d "merge" (tip)
    future parents are 11 and 7
-  rebase status stored
    already in target
    merge against 10:2f2496ddf49d
      detach base 9:e31216eec445
@@ -269,6 +267,7 @@  Check that the right ancestors is used w
   committing changelog
   rebased as 2a7f09cac94c
   rebase merging completed
+  rebase status stored
   update back to initial working directory parent
   resolving manifests
    branchmerge: False, force: False, partial: False