[PATCH 0/2] Simplify PythonParser

[PATCH 0/2] Simplify PythonParser

[Christopher Larson]

As you can see below, this simplifies the python parser by merging its nested
class into the main class, and by simplifying the name comparison logic. This
also seems to increase up front parse performance slightly, as can be seen
here, the result of wiping the cache and running bitbake -p repeatedly:

Old:
  77.72user 11.98system 1:12.54elapsed 123%CPU (0avgtext+0avgdata 509456maxresident)k
  0inputs+32136outputs (0major+132904minor)pagefaults 0swaps
  77.48user 12.73system 1:12.28elapsed 124%CPU (0avgtext+0avgdata 508704maxresident)k
  0inputs+32096outputs (0major+134062minor)pagefaults 0swaps
  80.30user 9.69system 1:04.96elapsed 138%CPU (0avgtext+0avgdata 511520maxresident)k
  0inputs+32112outputs (0major+132475minor)pagefaults 0swaps
  79.53user 11.29system 1:05.46elapsed 138%CPU (0avgtext+0avgdata 510704maxresident)k
  0inputs+32088outputs (0major+132093minor)pagefaults 0swaps
  78.89user 10.94system 1:09.31elapsed 129%CPU (0avgtext+0avgdata 505616maxresident)k
  0inputs+32136outputs (0major+132909minor)pagefaults 0swaps
  79.70user 10.41system 1:08.17elapsed 132%CPU (0avgtext+0avgdata 511632maxresident)k
  0inputs+32088outputs (0major+133901minor)pagefaults 0swaps

New:
  69.95user 17.44system 1:01.31elapsed 142%CPU (0avgtext+0avgdata 502032maxresident)k
  0inputs+32104outputs (0major+130402minor)pagefaults 0swaps
  75.36user 10.14system 1:04.88elapsed 131%CPU (0avgtext+0avgdata 504784maxresident)k
  0inputs+32096outputs (0major+133487minor)pagefaults 0swaps
  73.12user 12.70system 1:03.68elapsed 134%CPU (0avgtext+0avgdata 505440maxresident)k
  0inputs+32096outputs (0major+132893minor)pagefaults 0swaps
  74.68user 10.08system 1:04.64elapsed 131%CPU (0avgtext+0avgdata 522384maxresident)k
  0inputs+32056outputs (0major+132502minor)pagefaults 0swaps
  75.59user 9.59system 1:03.46elapsed 134%CPU (0avgtext+0avgdata 509760maxresident)k
  0inputs+32072outputs (0major+133315minor)pagefaults 0swaps
  75.33user 10.12system 1:02.48elapsed 136%CPU (0avgtext+0avgdata 509568maxresident)k
  0inputs+32104outputs (0major+133788minor)pagefaults 0swaps

The following changes since commit 8e4e75383e43d6da2c16ec5286186a0d0569b0f8:

  codeparser: make var_expands actually hold useful information (2011-10-27 22:45:52 -0700)

are available in the git repository at:
  https://github.com/kergoth/bitbake codeparser-cleanup

Christopher Larson (2):
      codeparser: merge the nested python parsing classes
      codeparser: simplify how we compare the called node names

 lib/bb/codeparser.py |  157 +++++++++++++++++--------------------------------
 1 files changed, 55 insertions(+), 102 deletions(-)

Christopher Larson (2):
  codeparser: merge the nested python parsing classes
  codeparser: simplify how we compare the called node names

 lib/bb/codeparser.py |  157 +++++++++++++++++--------------------------------
 1 files changed, 55 insertions(+), 102 deletions(-)

-- 
1.7.7

[PATCH 1/2] codeparser: merge the nested python parsing classes

[Christopher Larson]

The split is even less necessary now that we use ast.walk rather than an
actual NodeVisitor subclass.

Signed-off-by: Christopher Larson <kergoth@gmail.com>
---
 lib/bb/codeparser.py |  183 +++++++++++++++++++++++---------------------------
 1 files changed, 84 insertions(+), 99 deletions(-)

diff --git a/lib/bb/codeparser.py b/lib/bb/codeparser.py
index 0dd9a36..0f3d646 100644
--- a/lib/bb/codeparser.py
+++ b/lib/bb/codeparser.py
@@ -151,116 +151,103 @@ def parser_cache_savemerge(d):
 
 
 class PythonParser():
-    class ValueVisitor():
-        """Visitor to traverse a python abstract syntax tree and obtain
-        the variables referenced via bitbake metadata APIs, and the external
-        functions called.
-        """
+    getvars = ("d.getVar", "bb.data.getVar", "data.getVar")
+    expands = ("d.expand", "bb.data.expand", "data.expand")
+    execfuncs = ("bb.build.exec_func", "bb.build.exec_task")
 
-        getvars = ("d.getVar", "bb.data.getVar", "data.getVar")
-        expands = ("d.expand", "bb.data.expand", "data.expand")
-        execs = ("bb.build.exec_func", "bb.build.exec_task")
+    @classmethod
+    def _compare_name(cls, strparts, node):
+        """Given a sequence of strings representing a python name,
+        where the last component is the actual Name and the prior
+        elements are Attribute nodes, determine if the supplied node
+        matches.
+        """
 
-        @classmethod
-        def _compare_name(cls, strparts, node):
-            """Given a sequence of strings representing a python name,
-            where the last component is the actual Name and the prior
-            elements are Attribute nodes, determine if the supplied node
-            matches.
-            """
+        if not strparts:
+            return True
 
-            if not strparts:
+        current, rest = strparts[0], strparts[1:]
+        if isinstance(node, ast.Attribute):
+            if current == node.attr:
+                return cls._compare_name(rest, node.value)
+        elif isinstance(node, ast.Name):
+            if current == node.id:
                 return True
+        return False
 
-            current, rest = strparts[0], strparts[1:]
-            if isinstance(node, ast.Attribute):
-                if current == node.attr:
-                    return cls._compare_name(rest, node.value)
-            elif isinstance(node, ast.Name):
-                if current == node.id:
-                    return True
-            return False
-
-        @classmethod
-        def compare_name(cls, value, node):
-            """Convenience function for the _compare_node method, which
-            can accept a string (which is split by '.' for you), or an
-            iterable of strings, in which case it checks to see if any of
-            them match, similar to isinstance.
-            """
+    @classmethod
+    def compare_name(cls, value, node):
+        """Convenience function for the _compare_node method, which
+        can accept a string (which is split by '.' for you), or an
+        iterable of strings, in which case it checks to see if any of
+        them match, similar to isinstance.
+        """
 
-            if isinstance(value, basestring):
-                return cls._compare_name(tuple(reversed(value.split("."))),
-                                         node)
-            else:
-                return any(cls.compare_name(item, node) for item in value)
+        if isinstance(value, basestring):
+            return cls._compare_name(tuple(reversed(value.split("."))),
+                                        node)
+        else:
+            return any(cls.compare_name(item, node) for item in value)
 
-        def __init__(self, value):
-            self.var_references = set()
-            self.var_execs = set()
-            self.direct_func_calls = set()
-            self.var_expands = set()
-            self.value = value
+    @classmethod
+    def warn(cls, func, arg):
+        """Warn about calls of bitbake APIs which pass a non-literal
+        argument for the variable name, as we're not able to track such
+        a reference.
+        """
 
-        @classmethod
-        def warn(cls, func, arg):
-            """Warn about calls of bitbake APIs which pass a non-literal
-            argument for the variable name, as we're not able to track such
-            a reference.
-            """
+        try:
+            funcstr = codegen.to_source(func)
+            argstr = codegen.to_source(arg)
+        except TypeError:
+            logger.debug(2, 'Failed to convert function and argument to source form')
+        else:
+            logger.debug(1, "Warning: in call to '%s', argument '%s' is "
+                            "not a literal", funcstr, argstr)
 
-            try:
-                funcstr = codegen.to_source(func)
-                argstr = codegen.to_source(arg)
-            except TypeError:
-                logger.debug(2, 'Failed to convert function and argument to source form')
+    def visit_Call(self, node):
+        if self.compare_name(self.getvars, node.func):
+            if isinstance(node.args[0], ast.Str):
+                self.var_references.add(node.args[0].s)
             else:
-                logger.debug(1, "Warning: in call to '%s', argument '%s' is "
-                                "not a literal", funcstr, argstr)
-
-        def visit_Call(self, node):
-            if self.compare_name(self.getvars, node.func):
-                if isinstance(node.args[0], ast.Str):
-                    self.var_references.add(node.args[0].s)
-                else:
-                    self.warn(node.func, node.args[0])
-            elif self.compare_name(self.expands, node.func):
-                if isinstance(node.args[0], ast.Str):
-                    self.warn(node.func, node.args[0])
-                    self.var_expands.add(node.args[0].s)
-                elif isinstance(node.args[0], ast.Call) and \
-                     self.compare_name(self.getvars, node.args[0].func):
-                    pass
-                else:
-                    self.warn(node.func, node.args[0])
-            elif self.compare_name(self.execs, node.func):
-                if isinstance(node.args[0], ast.Str):
-                    self.var_execs.add(node.args[0].s)
-                else:
-                    self.warn(node.func, node.args[0])
-            elif isinstance(node.func, ast.Name):
-                self.direct_func_calls.add(node.func.id)
-            elif isinstance(node.func, ast.Attribute):
-                # We must have a qualified name.  Therefore we need
-                # to walk the chain of 'Attribute' nodes to determine
-                # the qualification.
-                attr_node = node.func.value
-                identifier = node.func.attr
-                while isinstance(attr_node, ast.Attribute):
-                    identifier = attr_node.attr + "." + identifier
-                    attr_node = attr_node.value
-                if isinstance(attr_node, ast.Name):
-                    identifier = attr_node.id + "." + identifier
-                self.direct_func_calls.add(identifier)
+                self.warn(node.func, node.args[0])
+        elif self.compare_name(self.expands, node.func):
+            if isinstance(node.args[0], ast.Str):
+                self.warn(node.func, node.args[0])
+                self.var_expands.add(node.args[0].s)
+            elif isinstance(node.args[0], ast.Call) and \
+                    self.compare_name(self.getvars, node.args[0].func):
+                pass
+            else:
+                self.warn(node.func, node.args[0])
+        elif self.compare_name(self.execfuncs, node.func):
+            if isinstance(node.args[0], ast.Str):
+                self.var_execs.add(node.args[0].s)
+            else:
+                self.warn(node.func, node.args[0])
+        elif isinstance(node.func, ast.Name):
+            self.execs.add(node.func.id)
+        elif isinstance(node.func, ast.Attribute):
+            # We must have a qualified name.  Therefore we need
+            # to walk the chain of 'Attribute' nodes to determine
+            # the qualification.
+            attr_node = node.func.value
+            identifier = node.func.attr
+            while isinstance(attr_node, ast.Attribute):
+                identifier = attr_node.attr + "." + identifier
+                attr_node = attr_node.value
+            if isinstance(attr_node, ast.Name):
+                identifier = attr_node.id + "." + identifier
+            self.execs.add(identifier)
 
     def __init__(self):
-        #self.funcdefs = set()
+        self.var_references = set()
+        self.var_execs = set()
         self.execs = set()
-        #self.external_cmds = set()
+        self.var_expands = set()
         self.references = set()
 
     def parse_python(self, node):
-
         h = hash(str(node))
 
         if h in pythonparsecache:
@@ -271,14 +258,12 @@ class PythonParser():
         code = compile(check_indent(str(node)), "<string>", "exec",
                        ast.PyCF_ONLY_AST)
 
-        visitor = self.ValueVisitor(code)
         for n in ast.walk(code):
             if n.__class__.__name__ == "Call":
-                visitor.visit_Call(n)
+                self.visit_Call(n)
 
-        self.references.update(visitor.var_references)
-        self.references.update(visitor.var_execs)
-        self.execs = visitor.direct_func_calls
+        self.references.update(self.var_references)
+        self.references.update(self.var_execs)
 
         pythonparsecache[h] = {}
         pythonparsecache[h]["refs"] = self.references
-- 
1.7.7

[PATCH 2/2] codeparser: simplify how we compare the called node names

[Christopher Larson]

With the previous method, using the compare_name methods, we split the
requested match name by '.', reversed it, then compared them piecemeal
during the node traversal. The new method walks the nodes and hands back
the name of what's being called, and then we check that. This also
consolidates the two different implementations of traversal of the
attribute/name nodes (one in compare_name, one for the execs).

Signed-off-by: Christopher Larson <kergoth@gmail.com>
---
 lib/bb/codeparser.py |   72 ++++++++++++++------------------------------------
 1 files changed, 20 insertions(+), 52 deletions(-)

diff --git a/lib/bb/codeparser.py b/lib/bb/codeparser.py
index 0f3d646..7715613 100644
--- a/lib/bb/codeparser.py
+++ b/lib/bb/codeparser.py
@@ -156,40 +156,6 @@ class PythonParser():
     execfuncs = ("bb.build.exec_func", "bb.build.exec_task")
 
     @classmethod
-    def _compare_name(cls, strparts, node):
-        """Given a sequence of strings representing a python name,
-        where the last component is the actual Name and the prior
-        elements are Attribute nodes, determine if the supplied node
-        matches.
-        """
-
-        if not strparts:
-            return True
-
-        current, rest = strparts[0], strparts[1:]
-        if isinstance(node, ast.Attribute):
-            if current == node.attr:
-                return cls._compare_name(rest, node.value)
-        elif isinstance(node, ast.Name):
-            if current == node.id:
-                return True
-        return False
-
-    @classmethod
-    def compare_name(cls, value, node):
-        """Convenience function for the _compare_node method, which
-        can accept a string (which is split by '.' for you), or an
-        iterable of strings, in which case it checks to see if any of
-        them match, similar to isinstance.
-        """
-
-        if isinstance(value, basestring):
-            return cls._compare_name(tuple(reversed(value.split("."))),
-                                        node)
-        else:
-            return any(cls.compare_name(item, node) for item in value)
-
-    @classmethod
     def warn(cls, func, arg):
         """Warn about calls of bitbake APIs which pass a non-literal
         argument for the variable name, as we're not able to track such
@@ -206,39 +172,41 @@ class PythonParser():
                             "not a literal", funcstr, argstr)
 
     def visit_Call(self, node):
-        if self.compare_name(self.getvars, node.func):
+        name = self.called_node_name(node.func)
+        if name in self.getvars:
             if isinstance(node.args[0], ast.Str):
                 self.var_references.add(node.args[0].s)
             else:
                 self.warn(node.func, node.args[0])
-        elif self.compare_name(self.expands, node.func):
+        elif name in self.expands:
             if isinstance(node.args[0], ast.Str):
                 self.warn(node.func, node.args[0])
                 self.var_expands.add(node.args[0].s)
             elif isinstance(node.args[0], ast.Call) and \
-                    self.compare_name(self.getvars, node.args[0].func):
+                 self.called_node_name(node.args[0].func) in self.getvars:
                 pass
             else:
                 self.warn(node.func, node.args[0])
-        elif self.compare_name(self.execfuncs, node.func):
+        elif name in self.execfuncs:
             if isinstance(node.args[0], ast.Str):
                 self.var_execs.add(node.args[0].s)
             else:
                 self.warn(node.func, node.args[0])
-        elif isinstance(node.func, ast.Name):
-            self.execs.add(node.func.id)
-        elif isinstance(node.func, ast.Attribute):
-            # We must have a qualified name.  Therefore we need
-            # to walk the chain of 'Attribute' nodes to determine
-            # the qualification.
-            attr_node = node.func.value
-            identifier = node.func.attr
-            while isinstance(attr_node, ast.Attribute):
-                identifier = attr_node.attr + "." + identifier
-                attr_node = attr_node.value
-            if isinstance(attr_node, ast.Name):
-                identifier = attr_node.id + "." + identifier
-            self.execs.add(identifier)
+        elif name and isinstance(node.func, (ast.Name, ast.Attribute)):
+            self.execs.add(name)
+
+    def called_node_name(self, node):
+        """Given a called node, return its original string form"""
+        components = []
+        while node:
+            if isinstance(node, ast.Attribute):
+                components.append(node.attr)
+                node = node.value
+            elif isinstance(node, ast.Name):
+                components.append(node.id)
+                return '.'.join(reversed(components))
+            else:
+                break
 
     def __init__(self):
         self.var_references = set()
-- 
1.7.7