Performance optimization: use re.sub() for variable expansion. (Han-Wen Nienhyus)

4 files changed, 43 insertions, 30 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 25603b3..79069fd 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -104,6 +104,11 @@ RELEASE 0.97 - XXX
   - Enhance the tests to guarantee persistence of ListOption
     values in saved options files.
 
+  From Han-Wen Nienhuys:
+
+  - Optimize variable expansion by using the re.sub() method (when
+    possible).
+
   From Gary Oberbrunner:
 
   - Add an Environment.Dump() method to print the contents of a
diff --git a/src/engine/SCons/Util.py b/src/engine/SCons/Util.py
index 984bb02..2c50ea1 100644
--- a/src/engine/SCons/Util.py
+++ b/src/engine/SCons/Util.py
@@ -527,6 +527,7 @@ _regex_remove = [ _rm, None, _remove ]
 #       "$"                     [single dollar sign]
 #
 _separate_args = re.compile(r'(\$[\$\(\)]|\$[_a-zA-Z][\.\w]*|\${[^}]*}|\s+|[^\s\$]+|\$)')
+special_exps = re.compile (r'(\$[\$\(\)]|\$[_a-zA-Z][\.\w]*|\${[^}]*})')
 
 # This regular expression is used to replace strings of multiple white
 # space characters in the string result from the scons_subst() function.
@@ -601,9 +602,9 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, dict=No
                 else:
                     return s
             elif is_List(s):
-                r = []
-                for l in s:
-                    r.append(self.conv(self.substitute(l, lvars)))
+                def func(l, conv=self.conv, substitute=self.substitute, lvars=lvars):
+                    return conv(substitute(l, lvars))
+                r = map(func, s)
                 return string.join(r)
             elif callable(s):
                 try:
@@ -629,14 +630,24 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, dict=No
             separate tokens.
             """
             if is_String(args) and not isinstance(args, CmdStringHolder):
-                args = _separate_args.findall(args)
-                result = []
-                for a in args:
-                    result.append(self.conv(self.expand(a, lvars)))
                 try:
-                    result = string.join(result, '')
+                    def sub_match(match, conv=self.conv, expand=self.expand, lvars=lvars):
+                        return conv(expand(match.group(1), lvars))
+                    result = special_exps.sub(sub_match, args)
                 except TypeError:
-                    pass
+                    # If the internal conversion routine doesn't return
+                    # strings (it could be overridden to return Nodes,
+                    # for example), then the re module will throw this
+                    # exception.  Back off to a slower, general-purpose
+                    # algorithm that works for all data types.
+                    args = _separate_args.findall(args)
+                    result = []
+                    for a in args:
+                        result.append(self.conv(self.expand(a, lvars)))
+                    try:
+                        result = string.join(result, '')
+                    except TypeError:
+                        pass
                 return result
             else:
                 return self.expand(args, lvars)
@@ -903,39 +914,33 @@ def scons_subst_once(strSubst, env, key):
     We do this with some straightforward, brute-force code here...
     """
     matchlist = ['$' + key, '${' + key + '}']
+    val = env.get(key, '')
+    def sub_match(match, val=val, matchlist=matchlist):
+        a = match.group(1)
+        if a in matchlist:
+            a = val
+        if is_List(a):
+            return string.join(map(str, a))
+        else:
+            return str(a)
+
     if is_List(strSubst):
         result = []
         for arg in strSubst:
             if is_String(arg):
                 if arg in matchlist:
-                    arg = env[key]
+                    arg = val
                     if is_List(arg):
                         result.extend(arg)
                     else:
                         result.append(arg)
                 else:
-                    r = []
-                    for a in _separate_args.findall(arg):
-                        if a in matchlist:
-                            a = env[key]
-                        if is_List(a):
-                            r.append(string.join(map(str, a)))
-                        else:
-                            r.append(str(a))
-                    result.append(string.join(r, ''))
+                    result.append(special_exps.sub(sub_match, arg))
             else:
                 result.append(arg)
         return result
     elif is_String(strSubst):
-        result = []
-        for a in _separate_args.findall(strSubst):
-            if a in matchlist:
-                a = env[key]
-            if is_List(a):
-                result.append(string.join(map(str, a)))
-            else:
-                result.append(str(a))
-        return string.join(result, '')
+        return special_exps.sub(sub_match, strSubst)
     else:
         return strSubst
 
diff --git a/src/engine/SCons/UtilTests.py b/src/engine/SCons/UtilTests.py
index 5ac6ec0..61142d5 100644
--- a/src/engine/SCons/UtilTests.py
+++ b/src/engine/SCons/UtilTests.py
@@ -68,6 +68,9 @@ class DummyEnv:
     def __getitem__(self, key):
         return self.dict[key]
 
+    def get(self, key, default):
+        return self.dict.get(key, default)
+
     def sig_dict(self):
         dict = self.dict.copy()
         dict["TARGETS"] = 'tsig'
diff --git a/test/errors.py b/test/errors.py
index 7e24fed..1652f88 100644
--- a/test/errors.py
+++ b/test/errors.py
@@ -190,7 +190,7 @@ env.subst('$foo.bar.3.0')
 
 test.run(status=2, stderr="""
 scons: \*\*\* Syntax error `invalid syntax( \(line 1\))?' trying to evaluate `\$foo\.bar\.3\.0'
-File "SConstruct", line 2, in \?
+File "[^"]+", line \d+, in \S+
 """)
 
 test.write('SConstruct', """\
@@ -200,7 +200,7 @@ env.subst_list('$foo.3.0.x')
 
 test.run(status=2, stderr="""
 scons: \*\*\* Syntax error `invalid syntax( \(line 1\))?' trying to evaluate `\$foo\.3\.0\.x'
-File "SConstruct", line 2, in \?
+File "[^"]+", line \d+, in \S+
 """)
 
 #Test syntax errors when trying to expand construction variables at build time: