Reuse the same temp index in a transaction

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>

diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 732858e6769eb1a5d39da469eb521d2d61294663..4446720602ac54c8dcb20ec28a313362070e3cae 100644 (file)
--- 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):
     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 = self.temp_index()

-        index.read_tree(ours)

         try:
         try:

-            try:
-                index.apply_treediff(base, theirs, quiet = True)
-                return index.write_tree()
-            except MergeException:
-                return None
+            result, index_tree = index.merge(base, ours, theirs)

         finally:
             index.delete()
         finally:
             index.delete()

+        return result

     def apply(self, tree, patch_text, quiet):
         """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
     def apply(self, tree, patch_text, quiet):
         """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


@@ -566,6 +547,46 @@ class Index(RunWithEnv):
         # to use --binary.
         self.apply(self.__repository.diff_tree(tree1, tree2, ['--full-index']),
                    quiet)
         # to use --binary.
         self.apply(self.__repository.diff_tree(tree1, tree2, ['--full-index']),
                    quiet)

+    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, quiet = True)
+            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)
     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 23321c7052c61460894d324cb0fa75812b26ae57..dd7aad24302d6cad5147aa20e916aedfb2c8be26 100644 (file)
--- 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."""
 
 """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 *
 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.__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):
     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)
                 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(
     def __checkout(self, tree, iw):
         if not self.__stack.head_top_equal():
             out.error(


@@ -239,7 +248,8 @@ class StackTransaction(object):
         base = oldparent.data.tree
         ours = cd.parent.data.tree
         theirs = cd.tree
         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:
         merge_conflict = False
         if not tree:
             if iw == None: