[StGit PATCH v2] Reuse the same temp index in a transaction

["Karl Hasselström" <kha@treskal.com>]

Instead of making a new temp index every time we need one, just keep
reusing the same one. And keep track of which tree is currently stored
in it -- if we do several consecutive successful pushes, it's always
going to be the "right" tree so that we don't have to call read-tree
before each patch application.

The motivation behind this change is of course that it makes things
faster.

(The same simple test as in the previous patch -- pushing 250 patches
in a 32k-file repository, with one file-level merge necessary per push
-- went from 0.36 to 0.19 seconds per patch with this patch applied.)

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

Here's the same patch, but with Repository.index_merge() instead
called Index.merge(). A much better fit, I think.

Also some doc changes compared to v1.

 stgit/lib/git.py         |   63 +++++++++++++++++++++++++++++++---------------
 stgit/lib/transaction.py |   12 ++++++++-
 2 files changed, 53 insertions(+), 22 deletions(-)


diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 9402606..8fb8bd2 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -459,31 +459,12 @@ class Repository(RunWithEnv):
     def set_head_ref(self, ref, msg):
         self.run(['git', 'symbolic-ref', '-m', msg, 'HEAD', ref]).no_output()
     def simple_merge(self, base, ours, theirs):
-        """Given three L{Tree}s, tries to do an in-index merge with a
-        temporary index. Returns the result L{Tree}, or None if the
-        merge failed (due to conflicts)."""
-        assert isinstance(base, Tree)
-        assert isinstance(ours, Tree)
-        assert isinstance(theirs, Tree)
-
-        # Take care of the really trivial cases.
-        if base == ours:
-            return theirs
-        if base == theirs:
-            return ours
-        if ours == theirs:
-            return ours
-
         index = self.temp_index()
-        index.read_tree(ours)
         try:
-            try:
-                index.apply_treediff(base, theirs)
-                return index.write_tree()
-            except MergeException:
-                return None
+            result, index_tree = index.merge(base, ours, theirs)
         finally:
             index.delete()
+        return result
     def apply(self, tree, patch_text):
         """Given a L{Tree} and a patch, will either return the new L{Tree}
         that results when the patch is applied, or None if the patch
@@ -563,6 +544,46 @@ class Index(RunWithEnv):
         # contains all involved objects; in other words, we don't have
         # to use --binary.
         self.apply(self.__repository.diff_tree(tree1, tree2, ['--full-index']))
+    def merge(self, base, ours, theirs, current = None):
+        """Use the index (and only the index) to do a 3-way merge of the
+        L{Tree}s C{base}, C{ours} and C{theirs}. The merge will either
+        succeed (in which case the first half of the return value is
+        the resulting tree) or fail cleanly (in which case the first
+        half of the return value is C{None}).
+
+        If C{current} is given (and not C{None}), it is assumed to be
+        the L{Tree} currently stored in the index; this information is
+        used to avoid having to read the right tree (either of C{ours}
+        and C{theirs}) into the index if it's already there. The
+        second half of the return value is the tree now stored in the
+        index, or C{None} if unknown. If the merge succeeded, this is
+        often the merge result."""
+        assert isinstance(base, Tree)
+        assert isinstance(ours, Tree)
+        assert isinstance(theirs, Tree)
+        assert current == None or isinstance(current, Tree)
+
+        # Take care of the really trivial cases.
+        if base == ours:
+            return (theirs, current)
+        if base == theirs:
+            return (ours, current)
+        if ours == theirs:
+            return (ours, current)
+
+        if current == theirs:
+            # Swap the trees. It doesn't matter since merging is
+            # symmetric, and will allow us to avoid the read_tree()
+            # call below.
+            ours, theirs = theirs, ours
+        if current != ours:
+            self.read_tree(ours)
+        try:
+            self.apply_treediff(base, theirs)
+            result = self.write_tree()
+            return (result, result)
+        except MergeException:
+            return (None, ours)
     def delete(self):
         if os.path.isfile(self.__filename):
             os.remove(self.__filename)
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index e47997e..74bc74d 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -1,6 +1,8 @@
 """The L{StackTransaction} class makes it possible to make complex
 updates to an StGit stack in a safe and convenient way."""
 
+import atexit
+
 from stgit import exception, utils
 from stgit.utils import any, all
 from stgit.out import *
@@ -84,6 +86,7 @@ class StackTransaction(object):
             self.__allow_conflicts = lambda trans: allow_conflicts
         else:
             self.__allow_conflicts = allow_conflicts
+        self.__temp_index = self.temp_index_tree = None
     stack = property(lambda self: self.__stack)
     patches = property(lambda self: self.__patches)
     def __set_applied(self, val):
@@ -97,6 +100,12 @@ class StackTransaction(object):
                 or self.patches[self.applied[0]].data.parent == val)
         self.__base = val
     base = property(lambda self: self.__base, __set_base)
+    @property
+    def temp_index(self):
+        if not self.__temp_index:
+            self.__temp_index = self.__stack.repository.temp_index()
+            atexit.register(self.__temp_index.delete)
+        return self.__temp_index
     def __checkout(self, tree, iw):
         if not self.__stack.head_top_equal():
             out.error(
@@ -238,7 +247,8 @@ class StackTransaction(object):
         base = oldparent.data.tree
         ours = cd.parent.data.tree
         theirs = cd.tree
-        tree = self.__stack.repository.simple_merge(base, ours, theirs)
+        tree, self.temp_index_tree = self.temp_index.merge(
+            base, ours, theirs, self.temp_index_tree)
         merge_conflict = False
         if not tree:
             if iw == None: