[PATCH 1 of 3] rebase: use one dirstateguard for entire rebase

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Mon Mar 20 10:14:31 EDT 2017


At Mon, 20 Mar 2017 22:17:54 +0900,
Yuya Nishihara wrote:
> 
> On Mon, 20 Mar 2017 20:14:20 +0900, FUJIWARA Katsunori wrote:
> > (Cc-ed to yuya, for confirmation)
> > 
> > At Sun, 19 Mar 2017 12:00:56 -0700,
> > Durham Goode wrote:
> > > 
> > > # HG changeset patch
> > > # User Durham Goode <durham at fb.com>
> > > # Date 1489949655 25200
> > > #      Sun Mar 19 11:54:15 2017 -0700
> > > # Node ID c2f68e8cd96f92c2fa240fe04e541c14c1bad3d8
> > > # Parent  75e4bae56068bec4f965773a2d5f4d272a8dc5b0
> > > rebase: use one dirstateguard for entire rebase
> > > 
> > > Recently we switched rebases to run the entire rebase inside a single
> > > transaction, which dramatically improved the speed of rebases in repos with
> > > large working copies. Let's also move the dirstate into a single dirstateguard
> > > to get the same benefits. This let's us avoid serializing the dirstate after
> > > each commit.
> > > 
> > > In a large repo, rebasing 27 commits is sped up by about 20%.
> > > 
> > > I believe the test changes are because us touching the dirstate gave the
> > > transaction something to actually rollback.
> > > 
> > > diff --git a/hgext/rebase.py b/hgext/rebase.py
> > > --- a/hgext/rebase.py
> > > +++ b/hgext/rebase.py
> > > @@ -475,12 +475,24 @@ class rebaseruntime(object):
> > >                  editopt = True
> > >              editor = cmdutil.getcommiteditor(edit=editopt, editform=editform)
> > >              revtoreuse = max(self.state)
> > > -            newnode = concludenode(repo, revtoreuse, p1, self.external,
> > > -                                   commitmsg=commitmsg,
> > > -                                   extrafn=_makeextrafn(self.extrafns),
> > > -                                   editor=editor,
> > > -                                   keepbranches=self.keepbranchesf,
> > > -                                   date=self.date)
> > > +            dsguard = dirstateguard.dirstateguard(repo, 'rebase')
> > > +            try:
> > > +                newnode = concludenode(repo, revtoreuse, p1, self.external,
> > > +                                       commitmsg=commitmsg,
> > > +                                       extrafn=_makeextrafn(self.extrafns),
> > > +                                       editor=editor,
> > > +                                       keepbranches=self.keepbranchesf,
> > > +                                       date=self.date)
> > > +                dsguard.close()
> > > +                release(dsguard)
> > > +            except error.InterventionRequired:
> > > +                dsguard.close()
> > > +                release(dsguard)
> > > +                raise
> > > +            except Exception:
> > > +                release(dsguard)
> > > +                raise
> > > +
> > >              if newnode is None:
> > >                  newrev = self.target
> > >              else:
> > 
> > This hunk just moves dirstateguard creation from concludenode() to
> > caller side.
> > 
> > Therefore, creation of dirstateguard at each concludenode() while
> > rebasing is not "one dirstateguard for entire rebase" but still
> > "dirstateguard per rebased-revision".
> > 
> > In addition to it, dirstateguard in this hunk should be redundant,
> > because we already know that another dirstategurad is certainly
> > created in outer scope (but in same transaction) as below hunk.
> 
> This is actually _two_ dirstateguards for entire rebase. One for
> _performrebase and the other for _finishrebase. I haven't investigated if
> we can move _finishrebase into the transaction.

Oops, sorry I misunderstood the context. The 1st hunk is applied not
for _performrebase() but for _finishrebase(). It seems OK. Sorry,
again.

-- 
----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp


More information about the Mercurial-devel mailing list