[PATCH 3 of 3] histedit: add histedit.singletransaction config option

[Augie Fackler]

On Wed, Mar 08, 2017 at 04:33:40PM -0800, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1489019264 28800
> #      Wed Mar 08 16:27:44 2017 -0800
> # Node ID 92225c3757e833d117c91d063cb8d42f6f355eef
> # Parent  265484e77411893d910f2eff4efe02cfe6abd5b7
> histedit: add histedit.singletransaction config option
>
> This adds an option (which defaults to False) to run entire histedits in a
> single transaction. This results in 20-25% faster histedits in large repos where
> transaction startup cost is expensive.
>
> I didn't want to enable this by default because it has some unfortunate side
> effects. For instance, if a pretxncommit hook throws midway through the
> histedit, it will rollback the entire histedit and lose any progress the user
> had made. Same if the user aborts editting a commit message. It's still worth
> turning this on for large repos, but probably not for normal sized repos.
>
> Long term, once we have inmemory merging, we could do the entire histedit in
> memory, without a transaction, then we could selectively rollback just parts of
> it in the event of an exception.
>
> Tested it by running the tests with
> `--extra-config-opt=histedit.singletransaction=True`. The only failure was
> related to the hook rollback issue I mention above.
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -168,6 +168,13 @@ the drop to be implicit for missing comm
>    [histedit]
>    dropmissing = True
>
> +By default, histedit will close the transaction after each action, so if there
> +are any errors the latest work is preserved. For performance purposes, you can
> +configure histedit to use a single transaction across the entire histedit::

I feel like this isn't up-front enough about the risks around hooks
causing you to lose your entire histedit. Can we strengthen this
warning somehow? (Or am I being too worried?)

> +
> +  [histedit]
> +  singletransaction = True
> +
>  """
>
>  from __future__ import absolute_import
> @@ -269,6 +276,7 @@ class histeditstate(object):
>          self.lock = lock
>          self.wlock = wlock
>          self.backupfile = None
> +        self.tr = None
>          if replacements is None:
>              self.replacements = []
>          else:
> @@ -1105,18 +1113,45 @@ def _continuehistedit(ui, repo, state):
>
>      total = len(state.actions)
>      pos = 0
> -    while state.actions:
> -        state.write()
> -        actobj = state.actions[0]
> -        pos += 1
> -        ui.progress(_("editing"), pos, actobj.torule(),
> -                    _('changes'), total)
> -        ui.debug('histedit: processing %s %s\n' % (actobj.verb,\
> -                                                   actobj.torule()))
> -        parentctx, replacement_ = actobj.run()
> -        state.parentctxnode = parentctx.node()
> -        state.replacements.extend(replacement_)
> -        state.actions.pop(0)
> +    state.tr = None
> +
> +    # Force an initial state file write, so the user can run --abort/continue
> +    # even if there's an exception before the first transaction serialize.
> +    state.write()
> +    try:
> +        # Don't use singletransaction by default since it rolls the entire
> +        # transaction back if an unexpected exception happens (like a
> +        # pretxncommit hook throws, or the user aborts the commit msg editor).
> +        if ui.configbool("histedit", "singletransaction", False):
> +            # Don't use a 'with' for the transaction, since actions may close
> +            # and reopen a transaction. For example, if the action executes an
> +            # external process it may choose to commit the transaction first.
> +            state.tr = repo.transaction('histedit')
> +
> +        while state.actions:
> +            state.write(tr=state.tr)
> +            actobj = state.actions[0]
> +            pos += 1
> +            ui.progress(_("editing"), pos, actobj.torule(),
> +                        _('changes'), total)
> +            ui.debug('histedit: processing %s %s\n' % (actobj.verb,\
> +                                                       actobj.torule()))
> +            parentctx, replacement_ = actobj.run()
> +            state.parentctxnode = parentctx.node()
> +            state.replacements.extend(replacement_)
> +            state.actions.pop(0)
> +
> +        if state.tr is not None:
> +            state.tr.close()
> +    except error.InterventionRequired:
> +        if state.tr is not None:
> +            state.tr.close()
> +        raise
> +    except Exception:
> +        if state.tr is not None:
> +            state.tr.abort()
> +        raise
> +
>      state.write()
>      ui.progress(_("editing"), None)
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel