Performance enhancements: use a more efficient splitext() method; cache source suffix computation; clean up code in MultiStepBuilder.__call__(); replicate some logic in scons_subst(). (Anthony Roach)

4 files changed, 89 insertions, 48 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 335a3b5..6779ddd 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -118,6 +118,11 @@ RELEASE 0.08 -
     one build comand, in which case the commands will not be executed
     simultaneously.
 
+  - Significant performance gains from not using our own version of
+    the inefficient stock os.path.splitext() method, caching source
+    suffix computation, code cleanup in MultiStepBuilder.__call__(),
+    and replicating some logic in scons_subst().
+
   From Zed Shaw:
 
   - Add an Append() method to Environments, to append values to
diff --git a/src/engine/SCons/Builder.py b/src/engine/SCons/Builder.py
index c4e8643..c1e93c2 100644
--- a/src/engine/SCons/Builder.py
+++ b/src/engine/SCons/Builder.py
@@ -82,9 +82,9 @@ class DictCmdGenerator:
     def __call__(self, source, target, env, **kw):
         ext = None
         for src in map(str, source):
-            my_ext = os.path.splitext(src)[1]
+            my_ext = SCons.Util.splitext(src)[1]
             if ext and my_ext != ext:
-                raise UserError("Cannot build multiple sources with different extensions.")
+                raise UserError("Cannot build multiple sources with different extensions: %s, %s" % (ext, my_ext))
             ext = my_ext
 
         if ext is None:
@@ -274,7 +274,7 @@ class BuilderBase:
                         path, fn = os.path.split(os.path.normpath(f))
                         f = os.path.join(path, pre + fn)
                     # Only append a suffix if the file does not have one.
-                    if suf and not os.path.splitext(f)[1]:
+                    if suf and not SCons.Util.splitext(f)[1]:
                         if f[-len(suf):] != suf:
                             f = f + suf
                 ret.append(f)
@@ -442,18 +442,18 @@ class MultiStepBuilder(BuilderBase):
         if not SCons.Util.is_List(src_builder):
             src_builder = [ src_builder ]
         self.src_builder = src_builder
-        self.sdict = {}
+        self.sdict = {} 
+        self.cached_src_suffixes = {} # source suffixes keyed on id(env)
 
     def __call__(self, env, target = None, source = None, **kw):
         slist = SCons.Node.arg2nodes(source, self.source_factory)
         final_sources = []
 
-        r=repr(env)
         try:
-            sdict = self.sdict[r]
+            sdict = self.sdict[id(env)]
         except KeyError:
             sdict = {}
-            self.sdict[r] = sdict
+            self.sdict[id(env)] = sdict
             for bld in self.src_builder:
                 if SCons.Util.is_String(bld):
                     try:
@@ -463,33 +463,27 @@ class MultiStepBuilder(BuilderBase):
                 for suf in bld.src_suffixes(env):
                     sdict[suf] = bld
                     
+        src_suffixes = self.src_suffixes(env)
         for snode in slist:
-            path, ext = os.path.splitext(snode.abspath)
+            path, ext = SCons.Util.splitext(snode.abspath)
             if sdict.has_key(ext):
                 src_bld = sdict[ext]
-
-                dictArgs = copy.copy(kw)
-                dictArgs['target'] = [path]
-                dictArgs['source'] = snode
-                dictArgs['env'] = env
-                tgt = apply(src_bld, (), dictArgs)
-                if not SCons.Util.is_List(tgt):
-                    tgt = [ tgt ]
-
+                tgt = apply(src_bld, (env, path, snode), kw)
                 # Only supply the builder with sources it is capable
                 # of building.
-                tgt = filter(lambda x,
-                             suf=self.src_suffixes(env):
-                             os.path.splitext(SCons.Util.to_String(x))[1] in \
-                             suf, tgt)
-                final_sources.extend(tgt)
+                if SCons.Util.is_List(tgt):
+                    tgt = filter(lambda x, suf=src_suffixes:
+                                 SCons.Util.splitext(SCons.Util.to_String(x))[1] in suf,
+                                 tgt)
+                if not SCons.Util.is_List(tgt):
+                    final_sources.append(tgt)
+                else:
+                    final_sources.extend(tgt)
             else:
                 final_sources.append(snode)
-        dictKwArgs = kw
-        dictKwArgs['target'] = target
-        dictKwArgs['source'] = final_sources
+
         return apply(BuilderBase.__call__,
-                     (self, env), dictKwArgs)
+                     (self, env, target, final_sources), kw)
 
     def get_src_builders(self, env):
         """Return all the src_builders for this Builder.
@@ -512,10 +506,14 @@ class MultiStepBuilder(BuilderBase):
         """Return a list of the src_suffix attributes for all
         src_builders of this Builder.
         """
-        suffixes = BuilderBase.src_suffixes(self, env)
-        for builder in self.get_src_builders(env):
-            suffixes.extend(builder.src_suffixes(env))
-        return suffixes
+        try:
+            return self.cached_src_suffixes[id(env)]
+        except KeyError:
+            suffixes = BuilderBase.src_suffixes(self, env)
+            for builder in self.get_src_builders(env):
+                suffixes.extend(builder.src_suffixes(env))
+            self.cached_src_suffixes[id(env)] = suffixes
+            return suffixes
 
 class CompositeBuilder(SCons.Util.Proxy):
     """A Builder Proxy whose main purpose is to always have
diff --git a/src/engine/SCons/Util.py b/src/engine/SCons/Util.py
index 38489ef..4c3a57f 100644
--- a/src/engine/SCons/Util.py
+++ b/src/engine/SCons/Util.py
@@ -47,6 +47,18 @@ except ImportError:
     class UserString:
         pass
 
+def splitext(path):
+    "Same as os.path.splitext() but faster."
+    if os.altsep:
+        sep = max(string.rfind(path, os.sep), string.rfind(path, os.altsep))
+    else:
+        sep = string.rfind(path, os.sep)
+    dot = string.rfind(path, '.')
+    if dot > sep:
+        return path[:dot],path[dot:]
+    else:
+        return path,""
+
 def updrive(path):
     """
     Make the drive letter (if any) upper case.
@@ -108,11 +120,11 @@ class PathList(UserList.UserList):
     def __getBasePath(self):
         """Return the file's directory and file name, with the
         suffix stripped."""
-        return self.__splitPath(os.path.splitext)[0]
+        return self.__splitPath(splitext)[0]
 
     def __getSuffix(self):
         """Return the file's suffix."""
-        return self.__splitPath(os.path.splitext)[1]
+        return self.__splitPath(splitext)[1]
 
     def __getFileName(self):
         """Return the file's name without the path."""
@@ -124,11 +136,11 @@ class PathList(UserList.UserList):
 
     def __getBase(self):
         """Return the file name with path and suffix stripped."""
-        return self.__getFileName().__splitPath(os.path.splitext)[0]
+        return self.__getFileName().__splitPath(splitext)[0]
 
     def __getAbsPath(self):
         """Return the absolute path"""
-	return map(lambda x: updrive(os.path.abspath(x)), self.data)
+        return map(lambda x: updrive(os.path.abspath(x)), self.data)
 
     dictSpecialAttrs = { "file" : __getFileName,
                          "base" : __getBasePath,
@@ -203,9 +215,9 @@ def scons_subst_list(strSubst, globals, locals, remove=None):
 
     def repl(m, globals=globals, locals=locals):
         key = m.group(1)
-        if key[:1] == '{' and key[-1:] == '}':
+        if key[0] == '{':
             key = key[1:-1]
-	try:
+        try:
             e = eval(key, globals, locals)
             if e is None:
                 s = ''
@@ -213,10 +225,9 @@ def scons_subst_list(strSubst, globals, locals, remove=None):
                 s = string.join(map(to_String, e), '\0')
             else:
                 s = _space_sep.sub('\0', to_String(e))
-	except NameError:
-	    s = ''
-	return s
-    n = 1
+        except NameError:
+            s = ''
+        return s
 
     if is_List(strSubst):
         # This looks like our input is a list of strings,
@@ -227,8 +238,9 @@ def scons_subst_list(strSubst, globals, locals, remove=None):
     else:
         # Tokenize the original string...
         strSubst = _space_sep.sub('\0', to_String(strSubst))
-    
+
     # Now, do the substitution
+    n = 1
     while n != 0:
         strSubst, n = _cv.subn(repl, strSubst)
     # Now parse the whole list into tokens.
@@ -248,14 +260,35 @@ def scons_subst(strSubst, globals, locals, remove=None):
     surrounded by curly braces to separate the name from
     trailing characters.
     """
+
+    # This function needs to be fast, so don't call scons_subst_list
+
+    def repl(m, globals=globals, locals=locals):
+        key = m.group(1)
+        if key[0] == '{':
+            key = key[1:-1]
+        try:
+            e = eval(key, globals, locals)
+            if e is None:
+                s = ''
+            elif is_List(e):
+                s = string.join(map(to_String, e), ' ')
+            else:
+                s = to_String(e)
+        except NameError:
+            s = ''
+        return s
+
+    # Now, do the substitution
+    n = 1
+    while n != 0:
+        strSubst,n = _cv.subn(repl, strSubst)
+    # and then remove remove
+    if remove:
+        strSubst = remove.sub('', strSubst)
     
-    # Make the common case (i.e. nothing to do) fast:
-    if string.find(strSubst, "$") == -1 \
-       and (remove is None or remove.search(strSubst) is None):
-        return strSubst
-    
-    cmd_list = scons_subst_list(strSubst, globals, locals, remove)
-    return string.join(map(string.join, cmd_list), '\n')
+    # strip out redundant white-space
+    return string.strip(_space_sep.sub(' ', strSubst))
 
 def render_tree(root, child_func, margin=[0], visited={}):
     """
diff --git a/src/engine/SCons/UtilTests.py b/src/engine/SCons/UtilTests.py
index 31dceb1..1f712cf 100644
--- a/src/engine/SCons/UtilTests.py
+++ b/src/engine/SCons/UtilTests.py
@@ -129,6 +129,11 @@ class UtilTestCase(unittest.TestCase):
         newcom = scons_subst("test $a $b $c $d test", glob, loc)
         assert newcom == "test 3 2 4 test", newcom
 
+    def test_splitext(self):
+        assert splitext('foo') == ('foo','')
+        assert splitext('foo.bar') == ('foo','.bar')
+        assert splitext(os.path.join('foo.bar', 'blat')) == (os.path.join('foo.bar', 'blat'),'')
+
     def test_subst_list(self):
         """Testing the scons_subst_list() method..."""
         loc = {}