Skip to contentSkip to conversation

Simplify PythonParser

PATCH New Christopher Larson 1 participant 3 messages Oct 28, 2011

lore
Share & tools
lore

Christopher Larson

author
#0 lore raw

cover Simplify PythonParser

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(-)
Signature · 1 line
1.7.7
author
#1 lore raw

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

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

Signed-off-by Christopher Larson <[email protected]>
---
 lib/bb/codeparser.py |  183 +++++++++++++++++++++++---------------------------
 1 files changed, 84 insertions(+), 99 deletions(-)
lib/bb/codeparser.py +84 -99
index 0dd9a36..0f3d646 100644
@@ -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
Signature · 1 line
1.7.7
author
#2 lore raw

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

In reply to 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 <[email protected]>
---
 lib/bb/codeparser.py |   72 ++++++++++++++------------------------------------
 1 files changed, 20 insertions(+), 52 deletions(-)
lib/bb/codeparser.py +20 -52
index 0f3d646..7715613 100644
@@ -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()
Signature · 1 line
1.7.7