Bug 1224411 - Speed up FileRegistry._partial_paths by memoizing on the basis of directory. r=nalexander
authorChris Manchester <cmanchester@mozilla.com>
Fri, 13 Nov 2015 16:14:40 -0800
changeset 272534 67191b9199fde82f791de6ddaafae40d4472da1e
parent 272533 ff761f8ebfcf02617d442c2a3a8e38ee29a9cecf
child 272535 4d4c786e522f03dd012d5c95b870b2f631c54aa7
push id29674
push userphilringnalda@gmail.com
push dateSat, 14 Nov 2015 21:22:32 +0000
treeherdermozilla-central@51fa3e0d4f7b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs1224411
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1224411 - Speed up FileRegistry._partial_paths by memoizing on the basis of directory. r=nalexander This function was found to be a little slow while profiling due to repeated calls to mozpath.dirname. This patch speeds up the function replacing dirname with string manipulation (these paths are already normalized), by caching results on the basis of directory, and converting from iteration to recursion to increase use of the cache. This commit speeds up the "install tests" step run as a part of the build and running tests by ~10% on a fast linux laptop.
python/mozbuild/mozpack/copier.py
python/mozbuild/mozpack/test/test_copier.py
--- a/python/mozbuild/mozpack/copier.py
+++ b/python/mozbuild/mozpack/copier.py
@@ -30,27 +30,33 @@ class FileRegistry(object):
 
         registry = FileRegistry()
         registry.add('foo/bar', file_instance)
     '''
 
     def __init__(self):
         self._files = OrderedDict()
         self._required_directories = Counter()
+        self._partial_paths_cache = {}
 
     def _partial_paths(self, path):
         '''
         Turn "foo/bar/baz/zot" into ["foo/bar/baz", "foo/bar", "foo"].
         '''
-        partial_paths = []
-        partial_path = path
-        while partial_path:
-            partial_path = mozpath.dirname(partial_path)
-            if partial_path:
-                partial_paths.append(partial_path)
+        dir_name = path.rpartition('/')[0]
+        if not dir_name:
+            return []
+
+        partial_paths = self._partial_paths_cache.get(dir_name)
+        if partial_paths:
+            return partial_paths
+
+        partial_paths = [dir_name] + self._partial_paths(dir_name)
+
+        self._partial_paths_cache[dir_name] = partial_paths
         return partial_paths
 
     def add(self, path, content):
         '''
         Add a BaseFile instance to the container, under the given path.
         '''
         assert isinstance(content, BaseFile)
         if path in self._files:
--- a/python/mozbuild/mozpack/test/test_copier.py
+++ b/python/mozbuild/mozpack/test/test_copier.py
@@ -33,16 +33,26 @@ class TestFileRegistry(MatchTestTemplate
     def do_check(self, pattern, result):
         self.checked = True
         if result:
             self.assertTrue(self.registry.contains(pattern))
         else:
             self.assertFalse(self.registry.contains(pattern))
         self.assertEqual(self.registry.match(pattern), result)
 
+    def test_partial_paths(self):
+        cases = {
+            'foo/bar/baz/zot': ['foo/bar/baz', 'foo/bar', 'foo'],
+            'foo/bar': ['foo'],
+            'bar': [],
+        }
+        reg = FileRegistry()
+        for path, parts in cases.iteritems():
+            self.assertEqual(reg._partial_paths(path), parts)
+
     def test_file_registry(self):
         self.registry = FileRegistry()
         self.registry.add('foo', GeneratedFile('foo'))
         bar = GeneratedFile('bar')
         self.registry.add('bar', bar)
         self.assertEqual(self.registry.paths(), ['foo', 'bar'])
         self.assertEqual(self.registry['bar'], bar)