[PATCH 3/N] Optimize CVSFile.filename

Jon Foster <[email protected]> Wed, 31 Mar 2010 13:25:26 +0100

Newsgroups	gmane.comp.version-control.subversion.cvs2svn.devel
Message-ID	<[email protected]>

Hi,

Selected parts of my profiling results:

ncalls cumtime function
     1    4183 main.py:run_with_profiling
     1    2997 collect_data.py:process_project
    2M    1957 collect_data.py:transform_symbol
   42M    1845 SubtreeSymbolTransform.__does_rule_apply_to
  234M     736 os.path.dirname
   42M     476 CVSFile.get_filename
   42M     285 os.path.normpath
   95M*    266 CVSDirectory.get_filename

(*including recursive calls, which were about 50% of the total).

So CVSFile.get_filename() is responsible for about 10% of cvs2svn's
overall runtime.  This patch reduces that to almost zero, by
calculating the filename when the CVSFile is created, and storing
it directly in a CVSFile.filename member variable.

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=2467690

To unsubscribe from this discussion, e-mail: [[email protected]].

cvs2svn_opt3_cvsfile_filename_patch.txt (text/plain, 2.1 KB)

--- cvs2svn_lib/cvs_path.py	2010-03-31 12:41:02.000000000 +0100
+++ cvs2svn_lib/cvs_path.py	2010-03-31 12:58:09.000000000 +0100
@@ -259,7 +259,8 @@
       'executable',
       'file_size',
       'mode',
-      'description'
+      'description',
+      'filename',
       ]
 
   def __init__(
@@ -274,9 +275,20 @@
     self.file_size = file_size
     self.mode = mode
     self.description = description
+    self._calculate_filename()
 
     assert self.parent_directory is not None
 
+  def _calculate_filename(self):
+    if self._in_attic:
+      self.filename = os.path.join(
+          self.parent_directory.filename, 'Attic', self.basename + ',v'
+          )
+    else:
+      self.filename = os.path.join(
+          self.parent_directory.filename, self.basename + ',v'
+          )
+
   def get_filename(self):
     """Return the filesystem path to this CVSPath in the CVS repository.
 
@@ -286,16 +298,19 @@
     It starts with the repository path passed to run_options.add_project()
     in the options.py file."""
 
-    if self._in_attic:
-      return os.path.join(
-          self.parent_directory.filename, 'Attic', self.basename + ',v'
-          )
-    else:
-      return os.path.join(
-          self.parent_directory.filename, self.basename + ',v'
-          )
+    # This turns out to be a hot path through the code.
+    # It's used by SubtreeSymbolTransform and similar transforms, so it's
+    # called at least:
+    #   (num_files * num_symbols_per_file * num_subtree_symbol_transforms)
+    # times.  On a large repository with several subtree symbol transforms,
+    # that can exceed 100,000,000 calls.  And _calculate_filename() is quite
+    # complex, so doing that every time could add about 10 minutes to the
+    # cvs2svn runtime.
+    #
+    # So now we precalculate this and just return it.
+    #
 
-  filename = property(get_filename)
+    return self.filename
 
   def __getstate__(self):
     return (
@@ -311,6 +326,7 @@
         self.description
         ) = state
     CVSPath.__setstate__(self, cvs_path_state)
+    self._calculate_filename()
 
   def __str__(self):
     """For convenience only.  The format is subject to change at any time."""