Performance optimization: parse variables as they are set, to reduce the total number of times things are parsed.
authorBenjamin Smedberg <benjamin@smedbergs.us>
Wed, 25 Feb 2009 15:30:29 -0500
changeset 178 e373af4da9fc
parent 177 38b633e97cf7
child 179 56f8cbd6b7b7
push id103
push userbsmedberg@mozilla.com
push date2009-02-25 21:20 +0000
Performance optimization: parse variables as they are set, to reduce the total number of times things are parsed. In the future, this should be done even more lazily.
pymake/data.py
--- a/pymake/data.py
+++ b/pymake/data.py
@@ -1,22 +1,18 @@
 """
 A representation of makefile data structures.
 """
 
 import logging, re, os
-import parserdata
-import pymake.parser
-import pymake.functions
-import pymake.process
-import pymake.util
+import parserdata, parser, functions, process, util
 
 _log = logging.getLogger('pymake.data')
 
-class DataError(pymake.util.MakeError):
+class DataError(util.MakeError):
     pass
 
 class ResolutionError(DataError):
     """
     Raised when dependency resolution fails, either due to recursion or to missing
     prerequisites.This is separately catchable so that implicit rule search can try things
     without having to commit.
     """
@@ -75,18 +71,23 @@ class Expansion(object):
         self.loc = loc
 
     @staticmethod
     def fromstring(s):
         e = Expansion()
         e.append(s)
         return e
 
+    def clone(self):
+        e = Expansion()
+        e._elements = list(self._elements)
+        return e
+
     def append(self, object):
-        if not isinstance(object, (str, pymake.functions.Function)):
+        if not isinstance(object, (str, functions.Function)):
             raise DataError("Expansions can contain only strings or functions, got %s" % (type(object),))
 
         if object == '':
             return
 
         if len(self) and isinstance(object, str) and isinstance(self[-1], str):
             self[-1] += object
         else:
@@ -162,34 +163,34 @@ class Variables(object):
     SOURCE_COMMANDLINE = 1
     SOURCE_MAKEFILE = 2
     SOURCE_ENVIRONMENT = 3
     SOURCE_AUTOMATIC = 4
     # I have no intention of supporting builtin rules or variables that go with them
     # SOURCE_IMPLICIT = 5
 
     def __init__(self, parent=None):
-        self._map = {}
+        self._map = {} # vname -> flavor, source, valuestr, valueexp, expansionerror
         self.parent = parent
 
     def readfromenvironment(self, env):
         for k, v in env.iteritems():
             self.set(k, self.FLAVOR_SIMPLE, self.SOURCE_ENVIRONMENT, v)
 
     def get(self, name, expand=True):
         """
         Get the value of a named variable. Returns a tuple (flavor, source, value)
 
         If the variable is not present, returns (None, None, None)
 
         @param expand If true, the value will be returned as an expansion. If false,
         it will be returned as an unexpanded string.
         """
         if name in self._map:
-            flavor, source, valuestr = self._map[name]
+            flavor, source, valuestr, valueexp, expansionerror = self._map[name]
             if flavor == self.FLAVOR_APPEND:
                 if self.parent:
                     pflavor, psource, pvalue = self.parent.get(name, expand)
                 else:
                     pflavor, psource, pvalue = None, None, None
 
                 if pvalue is None:
                     flavor = self.FLAVOR_RECURSIVE
@@ -197,30 +198,37 @@ class Variables(object):
                 else:
                     if source > psource:
                         # TODO: log a warning?
                         return pflavor, psource, pvalue
 
                     if not expand:
                         return pflavor, psource, pvalue + ' ' + valuestr
 
-                    d = pymake.parser.Data.fromstring(valuestr, parserdata.Location("Expansion of variable '%s'" % (name,), 1, 0))
-                    appende, t, o = pymake.parser.parsemakesyntax(d, 0, (), pymake.parser.iterdata)
+                    if expansionerror is not None:
+                        raise expansionerror
 
+                    
+                    d = parser.Data.fromstring(valuestr, parserdata.Location("Expansion of variable '%s'" % (name,), 1, 0))
+                    appende, t, o = parser.parsemakesyntax(d, 0, (), parser.iterdata)
+
+                    pvalue = pvalue.clone()
                     pvalue.append(' ')
-                    pvalue.concat(appende)
+                    pvalue.concat(valueexp)
 
                     return pflavor, psource, pvalue
                     
             if not expand:
                 return flavor, source, valuestr
 
             if flavor == self.FLAVOR_RECURSIVE:
-                d = pymake.parser.Data.fromstring(valuestr, parserdata.Location("Expansion of variable '%s'" % (name,), 1, 0))
-                val, t, o = pymake.parser.parsemakesyntax(d, 0, (), pymake.parser.iterdata)
+                if expansionerror is not None:
+                    raise expansionerror
+
+                val = valueexp
             else:
                 val = Expansion.fromstring(valuestr)
 
             return flavor, source, val
 
         if self.parent is not None:
             return self.parent.get(name, expand)
 
@@ -232,46 +240,79 @@ class Variables(object):
         assert isinstance(value, str), "expected str, got %s" % type(value)
 
         prevflavor, prevsource, prevvalue = self.get(name)
         if prevsource is not None and source > prevsource:
             # TODO: give a location for this warning
             _log.info("not setting variable '%s', set by higher-priority source to value '%s'" % (name, prevvalue))
             return
 
-        self._map[name] = (flavor, source, value)
+        if flavor == self.FLAVOR_SIMPLE:
+            valueexp = None
+            expansionerror = None
+        else:
+            try:
+                d = parser.Data.fromstring(value, parserdata.Location("Expansion of variable '%s'" % (name,), 1, 0))
+                valueexp, t, o = parser.parsemakesyntax(d, 0, (), parser.iterdata)
+                expansionerror = None
+            except parser.SyntaxError, e:
+                valueexp = None
+                expansionerror = e
+
+        self._map[name] = (flavor, source, value, valueexp, expansionerror)
 
     def append(self, name, source, value, variables, makefile):
         assert source in (self.SOURCE_OVERRIDE, self.SOURCE_MAKEFILE, self.SOURCE_AUTOMATIC)
         assert isinstance(value, str)
+
+        def expand():
+            try:
+                d = parser.Data.fromstring(value, parserdata.Location("Expansion of variable '%s'" % (name,), 1, 0))
+                valueexp, t, o = parser.parsemakesyntax(d, 0, (), parser.iterdata)
+                return valueexp, None
+            except parser.SyntaxError, e:
+                return None, e
         
-        if name in self._map:
-            prevflavor, prevsource, prevvalue = self._map[name]
-            if source > prevsource:
-                # TODO: log a warning?
-                return
+        if name not in self._map:
+            exp, err = expand()
+            self._map[name] = self.FLAVOR_APPEND, source, value, exp, err
+            return
+
+        prevflavor, prevsource, prevvalue, valueexp, err = self._map[name]
+        if source > prevsource:
+            # TODO: log a warning?
+            return
 
-            if prevflavor == self.FLAVOR_SIMPLE:
-                d = pymake.parser.Data.fromstring(value, parserdata.Location("Expansion of variable '%s'" % (name,), 1, 0))
-                e, t, o = pymake.parser.parsemakesyntax(d, 0, (), pymake.parser.iterdata)
-                val = e.resolve(makefile, variables, [name])
-            else:
-                val = value
+        if prevflavor == self.FLAVOR_SIMPLE:
+            exp, err = expand()
+            if err is not None:
+                raise err
+
+            val = exp.resolve(makefile, variables, [name])
+            self._map[name] = prevflavor, prevsource, prevvalue + ' ' + val, None, None
+            return
 
-            self._map[name] = prevflavor, prevsource, prevvalue + ' ' + val
-        else:
-            self._map[name] = self.FLAVOR_APPEND, source, value
+        newvalue = prevvalue + ' ' + value
+        try:
+            d = parser.Data.fromstring(newvalue, parserdata.Location("Expansion of variable '%s'" % (name,), 1, 0))
+            valueexp, t, o = parser.parsemakesyntax(d, 0, (), parser.iterdata)
+            err = None
+        except parser.SyntaxError, e:
+            valueexp = None
+            err = e
+
+        self._map[name] = prevflavor, prevsource, newvalue, valueexp, err
 
     def merge(self, other):
         assert isinstance(other, Variables)
         for k, flavor, source, value in other:
             self.set(k, flavor, source, value)
 
     def __iter__(self):
-        for k, (flavor, source, value) in self._map.iteritems():
+        for k, (flavor, source, value, valueexp, expansionerr) in self._map.iteritems():
             yield k, flavor, source, value
 
     def __contains__(self, item):
         return item in self._map
 
 class Pattern(object):
     """
     A pattern is a string, possibly with a % substitution character. From the GNU make manual:
@@ -697,17 +738,17 @@ class Target(object):
         assert self._state == MAKESTATE_NONE
 
         self._state = MAKESTATE_WORKING
         self._callbacks = [cb]
 
         indent = getindent(targetstack)
 
         # this object exists solely as a container to subvert python's read-only closures
-        o = pymake.util.makeobject(('unmadedeps', 'didanything', 'error'))
+        o = util.makeobject(('unmadedeps', 'didanything', 'error'))
 
         def iterdeps():
             for r, deps in _resolvedrules:
                 for d in deps:
                     yield d
 
         def startdep():
             try:
@@ -817,17 +858,17 @@ class Target(object):
 
             o.didanything = False
             o.unmadedeps = 1
             o.error = None
 
             depiterator = iterdeps()
             startdep()
 
-        except pymake.util.MakeError, e:
+        except util.MakeError, e:
             self._notifyerror(makefile, e)
 
 def dirpart(p):
     d, s, f = p.rpartition('/')
     if d == '':
         return '.'
 
     return d
@@ -901,17 +942,17 @@ class CommandWrapper(object):
     def _cb(self, res):
         if res != 0 and not self.ignoreErrors:
             self.usercb(error=DataError("command '%s' failed, return code %s" % (self.cline, res), self.loc))
         else:
             self.usercb(error=None)
 
     def __call__(self, cb):
         self.usercb = cb
-        pymake.process.call(self.cline, loc=self.loc, cb=self._cb, context=self.context, **self.kwargs)
+        process.call(self.cline, loc=self.loc, cb=self._cb, context=self.context, **self.kwargs)
 
 def getcommandsforrule(rule, target, makefile, prerequisites, stem):
     v = Variables(parent=target.variables)
     setautomaticvariables(v, makefile, target, prerequisites)
     if stem is not None:
         setautomatic(v, '*', [stem])
 
     env = makefile.getsubenvironment(v)
@@ -1153,17 +1194,17 @@ class Makefile(object):
     def include(self, path, required=True, loc=None):
         """
         Include the makefile at `path`.
         """
         self.included.append(path)
 
         fspath = os.path.join(self.workdir, path)
         if os.path.exists(fspath):
-            stmts = pymake.parser.parsefile(fspath)
+            stmts = parser.parsefile(fspath)
             self.variables.append('MAKEFILE_LIST', Variables.SOURCE_AUTOMATIC, path, None, self)
             stmts.execute(self)
             self.gettarget(path).explicit = True
         elif required:
             raise DataError("Attempting to include file '%s' which doesn't exist." % (path,), loc)
 
     def addvpath(self, pattern, dirs):
         """
@@ -1188,18 +1229,18 @@ class Makefile(object):
             if p.match(target):
                 vp.extend(dirs)
 
         return withoutdups(vp)
 
     def remakemakefiles(self, cb):
         reparse = False
 
-        o = pymake.util.makeobject(('remadecount',),
-                                   remadecount = 0)
+        o = util.makeobject(('remadecount',),
+                            remadecount = 0)
 
         def remakecb(error, didanything):
             if error is not None:
                 print "Error remaking makefiles (ignored): ", error
 
             o.remadecount += 1
             if o.remadecount == len(self.included):
                 assert len(mlist) == len(self.included)