[PATCH 1 of 3] rebase: use one dirstateguard for entire rebase
FUJIWARA Katsunori
foozy at lares.dti.ne.jp
Mon Mar 20 07:14:20 EDT 2017
(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.
IMHO, only two hunks below are enough for "one dirstateguard for
entire rebase".
> @@ -712,11 +724,19 @@ def rebase(ui, repo, **opts):
> return retcode
>
> with repo.transaction('rebase') as tr:
> + dsguard = dirstateguard.dirstateguard(repo, 'rebase')
> try:
> rbsrt._performrebase(tr)
> + dsguard.close()
> + release(dsguard)
> except error.InterventionRequired:
> + dsguard.close()
> + release(dsguard)
> tr.close()
> raise
> + except Exception:
> + release(dsguard)
> + raise
> rbsrt._finishrebase()
> finally:
> release(lock, wlock)
> @@ -840,33 +860,28 @@ def concludenode(repo, rev, p1, p2, comm
> '''Commit the wd changes with parents p1 and p2. Reuse commit info from rev
> but also store useful information in extra.
> Return node of committed revision.'''
> - dsguard = dirstateguard.dirstateguard(repo, 'rebase')
> - try:
> - repo.setparents(repo[p1].node(), repo[p2].node())
> - ctx = repo[rev]
> - if commitmsg is None:
> - commitmsg = ctx.description()
> - keepbranch = keepbranches and repo[p1].branch() != ctx.branch()
> - extra = {'rebase_source': ctx.hex()}
> - if extrafn:
> - extrafn(ctx, extra)
> + repo.setparents(repo[p1].node(), repo[p2].node())
> + ctx = repo[rev]
> + if commitmsg is None:
> + commitmsg = ctx.description()
> + keepbranch = keepbranches and repo[p1].branch() != ctx.branch()
> + extra = {'rebase_source': ctx.hex()}
> + if extrafn:
> + extrafn(ctx, extra)
>
> - targetphase = max(ctx.phase(), phases.draft)
> - overrides = {('phases', 'new-commit'): targetphase}
> - with repo.ui.configoverride(overrides, 'rebase'):
> - if keepbranch:
> - repo.ui.setconfig('ui', 'allowemptycommit', True)
> - # Commit might fail if unresolved files exist
> - if date is None:
> - date = ctx.date()
> - newnode = repo.commit(text=commitmsg, user=ctx.user(),
> - date=date, extra=extra, editor=editor)
> + targetphase = max(ctx.phase(), phases.draft)
> + overrides = {('phases', 'new-commit'): targetphase}
> + with repo.ui.configoverride(overrides, 'rebase'):
> + if keepbranch:
> + repo.ui.setconfig('ui', 'allowemptycommit', True)
> + # Commit might fail if unresolved files exist
> + if date is None:
> + date = ctx.date()
> + newnode = repo.commit(text=commitmsg, user=ctx.user(),
> + date=date, extra=extra, editor=editor)
>
> - repo.dirstate.setbranch(repo[newnode].branch())
> - dsguard.close()
> - return newnode
> - finally:
> - release(dsguard)
> + repo.dirstate.setbranch(repo[newnode].branch())
> + return newnode
>
> def rebasenode(repo, rev, p1, base, state, collapse, target):
> 'Rebase a single revision rev on top of p1 using base as merge ancestor'
> diff --git a/tests/test-rebase-collapse.t b/tests/test-rebase-collapse.t
> --- a/tests/test-rebase-collapse.t
> +++ b/tests/test-rebase-collapse.t
> @@ -572,6 +572,8 @@ Interactions between collapse and keepbr
> o 0: 'A'
>
> $ hg rebase --keepbranches --collapse -s 1 -d 3
> + transaction abort!
> + rollback completed
> abort: cannot collapse multiple named branches
> [255]
>
> diff --git a/tests/test-rebase-scenario-global.t b/tests/test-rebase-scenario-global.t
> --- a/tests/test-rebase-scenario-global.t
> +++ b/tests/test-rebase-scenario-global.t
> @@ -270,6 +270,8 @@ G onto B - merge revision with both pare
>
> $ hg rebase -s 6 -d 1
> rebasing 6:eea13746799a "G"
> + transaction abort!
> + rollback completed
> abort: cannot use revision 6 as base, result would have 3 parents
> [255]
> $ hg rebase --abort
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
--
----------------------------------------------------------------------
[FUJIWARA Katsunori] foozy at lares.dti.ne.jp
More information about the Mercurial-devel
mailing list