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

[Durham Goode]

On 3/8/17 4:33 PM, 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::
> +
> +  [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:

The bottom half of this patch is easier to read with diff -w below (but 
thunderbird tried to word wrap, so I've truncated some of the long lines 
for this paste):

@@ -1105,8 +1113,23 @@ def _continuehistedit(ui, repo, state):

      total = len(state.actions)
      pos = 0
+    state.tr = None
+
+    # Force an initial state file write, so the user can run
+    # even if there's an exception before the first transaction
+    state.write()
+    try:
+        # Don't use singletransaction by default since it rolls the
+        # transaction back if an unexpected exception happens (like a
+        # pretxncommit hook throws, or the user aborts the commit msg
+        if ui.configbool("histedit", "singletransaction", False):
+            # Don't use a 'with' for the transaction, since actions
+            # and reopen a transaction. For example, if the action + 
         # external process it may choose to commit the transaction
+            state.tr = repo.transaction('histedit')
+
      while state.actions:
-        state.write()
+            state.write(tr=state.tr)
          actobj = state.actions[0]
          pos += 1
          ui.progress(_("editing"), pos, actobj.torule(),
@@ -1117,6 +1140,18 @@ def _continuehistedit(ui, repo, state):
          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)