This is an archive of the discontinued Mercurial Phabricator instance.

Differential D134

rebase: add config to move rebase into a single transaction
ClosedPublic

Authored by durham on Jul 18 2017, 11:09 AM.

Details

Reviewers
martinvonz
Group Reviewers
hg-reviewers
Commits
rHGd341677d667d: rebase: add config to move rebase into a single transaction
Summary

This was previously landed as cf8ad0e6c0e4 but backed out in a5abaa81fa because
it broke hook mid rebase and caused conflict resolution data loss in the event
of unexpected exceptions. This new version adds the behavior back but behind a
config flag, since the performance improvement is notable in large repositories.

The next patch adds a test covering this config.

The old commit message was:

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.
(grafted from cca36c7f35261b0e31beb226bf361067ef0e06ab)
(grafted from dc497d8705b71503e32e07bd33925c1e42cf9c9a)

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durham created this revision.Jul 18 2017, 11:09 AM
martinvonz added a subscriber: martinvonz.Jul 18 2017, 11:29 AM
martinvonz added inline comments.
hgext/rebase.py
712–718

acceptintervention() also accepts None (it's a little unexpected, I know, but also convenient), so this can probably be:

tr = None
if singletr:
    tr = repo.transaction('rebase')  
with util.acceptintervention(tr):
    rbsrt._performrebase(tr)

Also get rid of nullcontextmanager() then of course

durham updated this revision to Diff 273.Jul 18 2017, 2:14 PM
durham marked an inline comment as done.Jul 18 2017, 2:15 PM
martinvonz accepted this revision.Jul 18 2017, 4:42 PM
This revision is now accepted and ready to land.Jul 18 2017, 4:42 PM
Closed by commit rHGd341677d667d: rebase: add config to move rebase into a single transaction (authored by durham). · Explain WhyJul 18 2017, 4:43 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/rebase.py (20 lines)

Diff 287

hgext/rebase.py

[self.dest], [self.dest],
inclusive=True) inclusive=True)
self.external = externalparent(self.repo, self.state, self.external = externalparent(self.repo, self.state,
self.destancestors) self.destancestors)
if dest.closesbranch() and not self.keepbranchesf: if dest.closesbranch() and not self.keepbranchesf:
self.ui.status(_('reopening closed branch head %s\n') % dest) 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 repo, ui, opts = self.repo, self.ui, self.opts
if self.keepbranchesf: if self.keepbranchesf:
# insert _savebranch at the start of extrafns so if # insert _savebranch at the start of extrafns so if
# there's a user-provided extrafn it can clobber branch if # there's a user-provided extrafn it can clobber branch if
# desired # desired
self.extrafns.insert(0, _savebranch) self.extrafns.insert(0, _savebranch)
if self.collapsef: if self.collapsef:
branches = set() branches = set()
pos += 1 pos += 1
ui.status(_('rebasing %s\n') % desc) ui.status(_('rebasing %s\n') % desc)
ui.progress(_("rebasing"), pos, ("%d:%s" % (rev, ctx)), ui.progress(_("rebasing"), pos, ("%d:%s" % (rev, ctx)),
_('changesets'), total) _('changesets'), total)
p1, p2, base = defineparents(repo, rev, self.dest, p1, p2, base = defineparents(repo, rev, self.dest,
self.state, self.state,
self.destancestors, self.destancestors,
self.obsoletenotrebased) self.obsoletenotrebased)
self.storestatus() self.storestatus(tr=tr)
storecollapsemsg(repo, self.collapsemsg) storecollapsemsg(repo, self.collapsemsg)
if len(repo[None].parents()) == 2: if len(repo[None].parents()) == 2:
repo.ui.debug('resuming interrupted rebase\n') repo.ui.debug('resuming interrupted rebase\n')
else: else:
try: try:
ui.setconfig('ui', 'forcemerge', opts.get('tool', ''), ui.setconfig('ui', 'forcemerge', opts.get('tool', ''),
'rebase') 'rebase')
stats = rebasenode(repo, rev, p1, base, self.state, stats = rebasenode(repo, rev, p1, base, self.state,
Configuration Options: Configuration Options:
You can make rebase require a destination if you set the following config You can make rebase require a destination if you set the following config
option:: option::
[commands] [commands]
rebase.requiredest = True rebase.requiredest = True
By default, rebase will close the transaction after each commit. For
performance purposes, you can configure rebase to use a single transaction
across the entire rebase. WARNING: This setting introduces a significant
risk of losing the work you've done in a rebase if the rebase aborts
unexpectedly::
[rebase]
singletransaction = True
Return Values: Return Values:
Returns 0 on success, 1 if nothing to rebase or there are Returns 0 on success, 1 if nothing to rebase or there are
unresolved conflicts. unresolved conflicts.
""" """
rbsrt = rebaseruntime(repo, ui, opts) rbsrt = rebaseruntime(repo, ui, opts)
return retcode return retcode
else: else:
dest, rebaseset = _definesets(ui, repo, destf, srcf, basef, revf, dest, rebaseset = _definesets(ui, repo, destf, srcf, basef, revf,
destspace=destspace) destspace=destspace)
retcode = rbsrt._preparenewrebase(dest, rebaseset) retcode = rbsrt._preparenewrebase(dest, rebaseset)
if retcode is not None: if retcode is not None:
return retcode return retcode
rbsrt._performrebase() tr = None
if ui.configbool('rebase', 'singletransaction'):
tr = repo.transaction('rebase')
with util.acceptintervention(tr):
rbsrt._performrebase(tr)
rbsrt._finishrebase() rbsrt._finishrebase()
martinvonzUnsubmitted
Done

acceptintervention() also accepts None (it's a little unexpected, I know, but also convenient), so this can probably be:

tr = None
if singletr:
    tr = repo.transaction('rebase')  
with util.acceptintervention(tr):
    rbsrt._performrebase(tr)

Also get rid of nullcontextmanager() then of course

martinvonz: acceptintervention() also accepts None (it's a little unexpected, I know, but also convenient)…
def _definesets(ui, repo, destf=None, srcf=None, basef=None, revf=None, def _definesets(ui, repo, destf=None, srcf=None, basef=None, revf=None,
destspace=None): destspace=None):
"""use revisions argument to define destination and rebase set """use revisions argument to define destination and rebase set
""" """
if revf is None: if revf is None:
revf = [] revf = []