[PATCH 4/4] Replace algorithm in SubtreeSymbolTransform
Jon Foster <[email protected]> Wed, 31 Mar 2010 15:12:36 +0100
| Newsgroups | gmane.comp.version-control.subversion.cvs2svn.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi, The algorithm used by SubtreeSymbolTransform.__does_rule_apply_to() was written to be simple and similar to the existing logic in SubtreeSymbolMapper.transform(). Unfortunately, profiling shows that it's extremely slow. So it's time to replace it with an algorithm written for speed. Kind regards, Jon ********************************************************************** This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd. If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone. Cabot Communications Limited Verona House, Filwood Road, Bristol BS16 3RY, UK +44 (0) 1179584232 Co. Registered in England number 02817269 Please contact the sender if you believe you have received this email in error. ********************************************************************** ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________ ------------------------------------------------------ http://cvs2svn.tigris.org/ds/viewMessage.do?dsForumId=1667&dsMessageId=2467757 To unsubscribe from this discussion, e-mail: [[email protected]].
cvs2svn_opt4_subtree_symbol_transform_patch.txt
(text/plain, 2.5 KB)
--- cvs2svn-trunk-opt1/cvs2svn_lib/symbol_transform.py 2010-03-31 12:41:02.000000000 +0100
+++ cvs2svn-trunk-optimize/cvs2svn_lib/symbol_transform.py 2010-03-31 14:55:45.000000000 +0100
@@ -251,19 +251,51 @@
assert isinstance(cvs_path, str)
self.__subtree = os.path.normcase(os.path.normpath(cvs_path))
+ self.__subtree_len = len(self.__subtree)
self.__inner = inner_symbol_transform
def __does_rule_apply_to(self, cvs_file):
+ #
+ # NOTE: This turns out to be a hot path through the code.
+ #
+ # It used to use logic similar to SubtreeSymbolMapper.transform(). And
+ # it used to take 44% of cvs2svn's total runtime on one real-world test.
+ # Now it's been optimized, it takes about 2%.
+ #
+ # This is called about:
+ # (num_files * num_symbols_per_file * num_subtree_symbol_transforms)
+ # times. On a large repository with several of these transforms,
+ # that can exceed 100,000,000 calls.
+ #
+
# cvs_file.filename is guaranteed to already be normalised the way
# os.path.normpath() normalises paths. So we don't need to call
- # os.path.normpath() again.
+ # os.path.normpath() again. (The os.path.normpath() function does
+ # quite a lot, so it's expensive).
+ #
+ # os.path.normcase is a no-op on POSIX systems (and therefore fast).
+ # Even on Windows it's only a memory allocation and case-change, it
+ # should be quite fast.
cvs_path = os.path.normcase(cvs_file.filename)
- while cvs_path != self.__subtree:
- new_cvs_path = os.path.dirname(cvs_path)
- if new_cvs_path == cvs_path:
- return False
- cvs_path = new_cvs_path
- return True
+
+ # Do most of the work in a single call, without allocating memory.
+ if not cvs_path.startswith(self.__subtree):
+ # Different prefix.
+ # This is the common "no match" case.
+ return False
+
+ if len(cvs_path) == self.__subtree_len:
+ # Exact match.
+ #
+ # This is quite rare, as self.__subtree is usually a directory and
+ # cvs_path is always a file.
+ return True
+
+ # We know cvs_path starts with self.__subtree, check the next character
+ # is a '/' (or if we're on Windows, a '\\'). If so, then cvs_path is a
+ # file under the self.__subtree directory tree, so we match. If not,
+ # then it's not a match.
+ return cvs_path[self.__subtree_len] == os.path.sep
def transform(self, cvs_file, symbol_name, revision):
if self.__does_rule_apply_to(cvs_file):