git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Karl Hasselström <kha@treskal.com>
Date
Jul 3, 2008, 21:38 UTC
Message-ID
<20080703213712.30489.51872.stgit@yoghurt>
In-Reply-To
<20080702061314.11361.28297.stgit@yoghurt>

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:
Previous: Karl HasselströmNext: Catalin Marinas
Message 5 of 9 in “push optimizations”
  1. 0/2 push optimizationsKarl Hasselström, Jul 2, 2008
  2. 1/2 Do simple in-index merge with diff+apply instead of read-treeKarl Hasselström, Jul 2, 2008
  3. Catalin MarinasJul 12, 2008
  4. 2/2 Reuse the same temp index in a transactionKarl Hasselström, Jul 2, 2008
  5. Reuse the same temp index in a transactionKarl Hasselström, Jul 3, 2008
  6. Catalin MarinasJul 12, 2008
  7. Karl HasselströmJul 14, 2008
  8. Catalin MarinasJul 7, 2008
  9. Karl HasselströmJul 8, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.