Cache get_suffix() and get_build_env(). (Kevin Quick)

6 files changed, 112 insertions, 37 deletions

diff --git a/src/engine/SCons/BuilderTests.py b/src/engine/SCons/BuilderTests.py
index 0ce451c..5cb07e8 100644
--- a/src/engine/SCons/BuilderTests.py
+++ b/src/engine/SCons/BuilderTests.py
@@ -948,9 +948,9 @@ class BuilderTestCase(unittest.TestCase):
         src = tgt.sources[0]
         assert tgt.builder.target_scanner != scanner, tgt.builder.target_scanner
         assert tgt.builder.source_scanner is None, tgt.builder.source_scanner
-        assert tgt.get_source_scanner(bar_y) is None, tgt.get_source_scanner(bar_y)
+        assert tgt.get_source_scanner(bar_y, env1) is None, tgt.get_source_scanner(bar_y, env1)
         assert not src.has_builder(), src.has_builder()
-        assert src.get_source_scanner(bar_y) is None, src.get_source_scanner(bar_y)
+        assert src.get_source_scanner(bar_y, env1) is None, src.get_source_scanner(bar_y, env1)
 
         # An Environment that has suffix-specified SCANNERS should
         # provide a source scanner to the target.
@@ -974,10 +974,10 @@ class BuilderTestCase(unittest.TestCase):
         src = tgt.sources[0]
         assert tgt.builder.target_scanner != scanner, tgt.builder.target_scanner
         assert not tgt.builder.source_scanner, tgt.builder.source_scanner
-        assert tgt.get_source_scanner(bar_y), tgt.get_source_scanner(bar_y)
-        assert str(tgt.get_source_scanner(bar_y)) == 'EnvTestScanner', tgt.get_source_scanner(bar_y)
+        assert tgt.get_source_scanner(bar_y, env3), tgt.get_source_scanner(bar_y, env3)
+        assert str(tgt.get_source_scanner(bar_y, env3)) == 'EnvTestScanner', tgt.get_source_scanner(bar_y, env3)
         assert not src.has_builder(), src.has_builder()
-        assert src.get_source_scanner(bar_y) is None, src.get_source_scanner(bar_y)
+        assert src.get_source_scanner(bar_y, env3) is None, src.get_source_scanner(bar_y, env3)
 
         # Can't simply specify the scanner as a builder argument; it's
         # global to all invocations of this builder.
@@ -985,10 +985,10 @@ class BuilderTestCase(unittest.TestCase):
         src = tgt.sources[0]
         assert tgt.builder.target_scanner != scanner, tgt.builder.target_scanner
         assert not tgt.builder.source_scanner, tgt.builder.source_scanner
-        assert tgt.get_source_scanner(bar_y), tgt.get_source_scanner(bar_y)
-        assert str(tgt.get_source_scanner(bar_y)) == 'EnvTestScanner', tgt.get_source_scanner(bar_y)
+        assert tgt.get_source_scanner(bar_y, env3), tgt.get_source_scanner(bar_y, env3)
+        assert str(tgt.get_source_scanner(bar_y, env3)) == 'EnvTestScanner', tgt.get_source_scanner(bar_y, env3)
         assert not src.has_builder(), src.has_builder()
-        assert src.get_source_scanner(bar_y) is None, src.get_source_scanner(bar_y)
+        assert src.get_source_scanner(bar_y, env3) is None, src.get_source_scanner(bar_y, env3)
 
         # Now use a builder that actually has scanners and ensure that
         # the target is set accordingly (using the specified scanner
@@ -1002,11 +1002,11 @@ class BuilderTestCase(unittest.TestCase):
         assert tgt.builder.source_scanner, tgt.builder.source_scanner
         assert tgt.builder.source_scanner == scanner, tgt.builder.source_scanner
         assert str(tgt.builder.source_scanner) == 'TestScanner', str(tgt.builder.source_scanner)
-        assert tgt.get_source_scanner(bar_y), tgt.get_source_scanner(bar_y)
-        assert tgt.get_source_scanner(bar_y) == scanner, tgt.get_source_scanner(bar_y)
-        assert str(tgt.get_source_scanner(bar_y)) == 'TestScanner', tgt.get_source_scanner(bar_y)
+        assert tgt.get_source_scanner(bar_y, env3), tgt.get_source_scanner(bar_y, env3)
+        assert tgt.get_source_scanner(bar_y, env3) == scanner, tgt.get_source_scanner(bar_y, env3)
+        assert str(tgt.get_source_scanner(bar_y, env3)) == 'TestScanner', tgt.get_source_scanner(bar_y, env3)
         assert not src.has_builder(), src.has_builder()
-        assert src.get_source_scanner(bar_y) is None, src.get_source_scanner(bar_y)
+        assert src.get_source_scanner(bar_y, env3) is None, src.get_source_scanner(bar_y, env3)
 
 
 
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index 1af739f..60ebb79 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -485,7 +485,11 @@ class Base(SCons.Node.Node):
         return self.dir
 
     def get_suffix(self):
-        return SCons.Util.splitext(self.name)[1]
+        try:
+            return self.ext
+        except AttributeError:
+            self.ext = SCons.Util.splitext(self.name)[1]
+            return self.ext
 
     def rfile(self):
         return self
diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index 12224d3..335c1a5 100644
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -801,7 +801,7 @@ class NodeTestCase(unittest.TestCase):
         """
         target = SCons.Node.Node()
         source = SCons.Node.Node()
-        s = target.get_source_scanner(source)
+        s = target.get_source_scanner(source, None)
         assert s is None, s
 
         ts1 = Scanner()
@@ -820,19 +820,19 @@ class NodeTestCase(unittest.TestCase):
         builder = Builder2(ts1)
             
         targets = builder([source])
-        s = targets[0].get_source_scanner(source)
+        s = targets[0].get_source_scanner(source, None)
         assert s is ts1, s
 
         target.builder_set(Builder2(ts1))
         target.builder.source_scanner = ts2
-        s = target.get_source_scanner(source)
+        s = target.get_source_scanner(source, None)
         assert s is ts2, s
 
         builder = Builder1(env=Environment(SCANNERS = [ts3]))
 
         targets = builder([source])
         
-        s = targets[0].get_source_scanner(source)
+        s = targets[0].get_source_scanner(source, builder.env)
         assert s is ts3, s
 
 
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 767b9e3..abbdf87 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -148,8 +148,17 @@ class Node:
 
     def get_build_env(self):
         """Fetch the appropriate Environment to build this node."""
-        executor = self.get_executor()
-        return executor.get_build_env()
+        try:
+            build_env = self._build_env
+        except AttributeError:
+            # This gets called a lot, so cache it. A node gets created
+            # in the context of a specific environment and it doesn't
+            # get "moved" to a different environment, so caching this
+            # value is safe.
+            executor = self.get_executor()
+            build_env = executor.get_build_env()
+            self._build_env = build_env
+        return self._build_env
 
     def set_executor(self, executor):
         """Set the action executor for this node."""
@@ -295,6 +304,7 @@ class Node:
 
     def builder_set(self, builder):
         self.builder = builder
+        self._src_scanners = {}  # cached scanners are based on the builder
 
     def has_builder(self):
         """Return whether this Node has a builder or not.
@@ -421,23 +431,57 @@ class Node:
             self.implicit_factory_cache[path] = n
             return n
 
-    def get_source_scanner(self, node):
+    def get_source_scanner(self, node, build_env):
         """Fetch the source scanner for the specified node
 
         NOTE:  "self" is the target being built, "node" is
         the source file for which we want to fetch the scanner.
+
+        build_env is the build environment (it's self.get_build_env(),
+                  but the caller always knows this so it can give it
+                  to us).
+
+        Implies self.has_builder() is true; again, expect to only be
+        called from locations where this is already verified.
+
+        This function may be called very often; it attempts to cache
+        the scanner found to improve performance.
         """
+        # Called from scan() for each child (node) of this node
+        # (self).  The scan() may be called multiple times, so this
+        # gets called a multiple of those times; caching results is
+        # good.  Index results based on the id of the child; can
+        # ignore build_env parameter for the index because it's passed
+        # as an optimization of an already-determined value, not as a
+        # changing parameter.
+
+        key = str(id(node)) + '|' + str(id(build_env))
+        try:
+            return self._src_scanners[key]
+        except AttributeError:
+            self._src_scanners = {}
+        except KeyError:
+            pass
+
         if not self.has_builder():
-            return None  # if not buildable, can't have sources...
+            self._src_scanners[key] = None
+            return None
+        
         try:
             scanner = self.builder.source_scanner
             if scanner:
+                self._src_scanners[key] = scanner
                 return scanner
         except AttributeError:
             pass
 
-        # No scanner specified by builder, try env['SCANNERS']
-        return self.get_build_env().get_scanner(node.scanner_key())
+        # Not cached, so go look up a scanner from env['SCANNERS']
+        # based on the node's scanner key (usually the file
+        # extension).
+        
+        scanner = build_env.get_scanner(node.scanner_key())
+        self._src_scanners[key] = scanner
+        return scanner
 
     def scan(self):
         """Scan this node's dependents for implicit dependencies."""
@@ -473,7 +517,7 @@ class Node:
                     self.del_binfo()
 
         for child in self.children(scan=0):
-            scanner = self.get_source_scanner(child)
+            scanner = self.get_source_scanner(child, build_env)
             if scanner:
                 deps = child.get_implicit_deps(build_env, scanner, self)
                 self._add_child(self.implicit, self.implicit_dict, deps)
@@ -833,7 +877,7 @@ class Node:
         if self.is_derived() and self.env:
             env = self.get_build_env()
             for s in self.sources:
-                scanner = self.get_source_scanner(s)
+                scanner = self.get_source_scanner(s, env)
                 def f(node, env=env, scanner=scanner, target=self):
                     return node.get_found_includes(env, scanner, target)
                 return SCons.Util.render_tree(s, f, 1)
diff --git a/src/engine/SCons/Scanner/ScannerTests.py b/src/engine/SCons/Scanner/ScannerTests.py
index 6520788..00ad7fb 100644
--- a/src/engine/SCons/Scanner/ScannerTests.py
+++ b/src/engine/SCons/Scanner/ScannerTests.py
@@ -399,10 +399,18 @@ class ClassicTestCase(unittest.TestCase):
         env = DummyEnvironment()
         s = MyScanner("t", ['.suf'], 'MYPATH', '^my_inc (\S+)')
 
+        # This set of tests is intended to test the scanning operation
+        # of the Classic scanner.
+
+        # Note that caching has been added for not just the includes
+        # but the entire scan call.  The caching is based on the
+        # arguments, so we will fiddle with the path parameter to
+        # defeat this caching for the purposes of these tests.
+
         # If the node doesn't exist, scanning turns up nothing.
         n1 = MyNode("n1")
         n1._exists = None
-        ret = s.scan(n1, env)
+        ret = s.function(n1, env)
         assert ret == [], ret
 
         # Verify that it finds includes from the contents.
@@ -410,22 +418,27 @@ class ClassicTestCase(unittest.TestCase):
         n._exists = 1
         n._dir = MyNode("n._dir")
         n._contents = 'my_inc abc\n'
-        ret = s.scan(n, env)
+        ret = s.function(n, env, ('foo',))
         assert ret == ['abc'], ret
 
         # Verify that it uses the cached include info.
         n._contents = 'my_inc def\n'
-        ret = s.scan(n, env)
+        ret = s.function(n, env, ('foo2',))
         assert ret == ['abc'], ret
 
         # Verify that if we wipe the cache, it uses the new contents.
         n.includes = None
-        ret = s.scan(n, env)
+        ret = s.function(n, env, ('foo3',))
         assert ret == ['def'], ret
 
+        # Verify that overall scan results are cached even if individual
+        # results are de-cached
+        ret = s.function(n, env, ('foo2',))
+        assert ret == ['abc'], ret
+
         # Verify that it sorts what it finds.
         n.includes = ['xyz', 'uvw']
-        ret = s.scan(n, env)
+        ret = s.function(n, env, ('foo4',))
         assert ret == ['uvw', 'xyz'], ret
 
         # Verify that we use the rfile() node.
@@ -434,9 +447,11 @@ class ClassicTestCase(unittest.TestCase):
         nr._dir = MyNode("nr._dir")
         nr.includes = ['jkl', 'mno']
         n._rfile = nr
-        ret = s.scan(n, env)
+        ret = s.function(n, env, ('foo5',))
         assert ret == ['jkl', 'mno'], ret
 
+        
+
 class ClassicCPPTestCase(unittest.TestCase):
     def test_find_include(self):
         """Test the Scanner.ClassicCPP find_include() method"""
diff --git a/src/engine/SCons/Scanner/__init__.py b/src/engine/SCons/Scanner/__init__.py
index e5ac2c6..cbab50c 100644
--- a/src/engine/SCons/Scanner/__init__.py
+++ b/src/engine/SCons/Scanner/__init__.py
@@ -296,8 +296,24 @@ class Classic(Current):
 
         self.cre = re.compile(regex, re.M)
         self.fs = fs
+        self._cached = {}
 
-        kw['function'] = self.scan
+        def _scan(node, env, path=(), self=self):
+            node = node.rfile()
+
+            if not node.exists():
+                return []
+
+            key = str(id(node)) + '|' + string.join(map(str, path), ':')
+            try:
+                return self._cached[key]
+            except KeyError:
+                pass
+
+            self._cached[key] = scan_result = self.scan(node, path)
+            return scan_result
+
+        kw['function'] = _scan
         kw['path_function'] = FindPathDirs(path_variable, fs)
         kw['recursive'] = 1
         kw['skeys'] = suffixes
@@ -314,11 +330,7 @@ class Classic(Current):
     def sort_key(self, include):
         return SCons.Node.FS._my_normcase(include)
 
-    def scan(self, node, env, path=()):
-        node = node.rfile()
-
-        if not node.exists():
-            return []
+    def scan(self, node, path=()):
 
         # cache the includes list in node so we only scan it once:
         if node.includes != None: