[StGit PATCH v2] 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>
---
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(-)
Show changes to 2 files +53 −22
stgit/lib/git.py, stgit/lib/transaction.py
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:
Karl Hasselström