[1.18][PATCH] Backport methodpool changes from master

[1.18][PATCH] Backport methodpool changes from master

[Denys Dmytriyenko]

From: Denys Dmytriyenko <denys@ti.com>

The patch to retire methodpool helps fix intermittent parsing issue in Dylan:
http://thread.gmane.org/gmane.comp.handhelds.openembedded.core/42786

Richard Purdie (1):
  methodpool: Retire it, remove global method scope

 lib/bb/cooker.py                   |  2 ++
 lib/bb/methodpool.py               | 43 --------------------------------------
 lib/bb/parse/ast.py                |  8 +++----
 lib/bb/parse/parse_py/BBHandler.py |  4 ----
 4 files changed, 5 insertions(+), 52 deletions(-)

-- 
1.8.3.2

[1.18][PATCH] methodpool: Retire it, remove global method scope

[Denys Dmytriyenko]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

Having a global method scope confuses users and with the introduction
of parallel parsing, its not even possible to correctly detect conflicting
functions. Rather than try and fix that, its simpler to retire the global
method scope and restrict functions to those locations they're defined
within. This is more what users actually expect too.

If we remove the global function scope, the need for methodpool is reduced
to the point we may as well retire it. There is some small loss of caching
of parsed functions but timing measurements so the impact to be neglibile
in the overall parsing time.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Tested-by: Denys Dmytriyenko <denys@ti.com>
---
 lib/bb/cooker.py                   |  2 ++
 lib/bb/methodpool.py               | 43 --------------------------------------
 lib/bb/parse/ast.py                |  8 +++----
 lib/bb/parse/parse_py/BBHandler.py |  4 ----
 4 files changed, 5 insertions(+), 52 deletions(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 2c54209..4d6cf81 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -1606,6 +1606,7 @@ class Parser(multiprocessing.Process):
         self.quit = quit
         self.init = init
         multiprocessing.Process.__init__(self)
+        self.context = bb.utils._context.copy()
 
     def run(self):
         if self.init:
@@ -1640,6 +1641,7 @@ class Parser(multiprocessing.Process):
 
     def parse(self, filename, appends, caches_array):
         try:
+            bb.utils._context = self.context.copy()
             return True, bb.cache.Cache.parse(filename, appends, self.cfg, caches_array)
         except Exception as exc:
             tb = sys.exc_info()[2]
diff --git a/lib/bb/methodpool.py b/lib/bb/methodpool.py
index 2fb5d96..3cf2040 100644
--- a/lib/bb/methodpool.py
+++ b/lib/bb/methodpool.py
@@ -17,24 +17,7 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
-
-"""
-    What is a method pool?
-
-    BitBake has a global method scope where .bb, .inc and .bbclass
-    files can install methods. These methods are parsed from strings.
-    To avoid recompiling and executing these string we introduce
-    a method pool to do this task.
-
-    This pool will be used to compile and execute the functions. It
-    will be smart enough to
-"""
-
 from bb.utils import better_compile, better_exec
-from bb       import error
-
-# A dict of function names we have seen
-_parsed_fns     = { }
 
 def insert_method(modulename, code, fn):
     """
@@ -43,29 +26,3 @@ def insert_method(modulename, code, fn):
     """
     comp = better_compile(code, modulename, fn )
     better_exec(comp, None, code, fn)
-
-    # now some instrumentation
-    code = comp.co_names
-    for name in code:
-        if name in ['None', 'False']:
-            continue
-        elif name in _parsed_fns and not _parsed_fns[name] == modulename:
-            error("The function %s defined in %s was already declared in %s. BitBake has a global python function namespace so shared functions should be declared in a common include file rather than being duplicated, or if the functions are different, please use different function names." % (name, modulename, _parsed_fns[name]))
-        else:
-            _parsed_fns[name] = modulename
-
-# A dict of modules the parser has finished with
-_parsed_methods = {}
-
-def parsed_module(modulename):
-    """
-    Has module been parsed?
-    """
-    return modulename in _parsed_methods
-
-def set_parsed_module(modulename):
-    """
-    Set module as parsed
-    """
-    _parsed_methods[modulename] = True
-
diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py
index b2657f8..713bef1 100644
--- a/lib/bb/parse/ast.py
+++ b/lib/bb/parse/ast.py
@@ -148,9 +148,8 @@ class MethodNode(AstNode):
         text = '\n'.join(self.body)
         if self.func_name == "__anonymous":
             funcname = ("__anon_%s_%s" % (self.lineno, self.filename.translate(string.maketrans('/.+-', '____'))))
-            if not funcname in bb.methodpool._parsed_fns:
-                text = "def %s(d):\n" % (funcname) + text
-                bb.methodpool.insert_method(funcname, text, self.filename)
+            text = "def %s(d):\n" % (funcname) + text
+            bb.methodpool.insert_method(funcname, text, self.filename)
             anonfuncs = data.getVar('__BBANONFUNCS') or []
             anonfuncs.append(funcname)
             data.setVar('__BBANONFUNCS', anonfuncs)
@@ -171,8 +170,7 @@ class PythonMethodNode(AstNode):
         # 'this' file. This means we will not parse methods from
         # bb classes twice
         text = '\n'.join(self.body)
-        if not bb.methodpool.parsed_module(self.modulename):
-            bb.methodpool.insert_method(self.modulename, text, self.filename)
+        bb.methodpool.insert_method(self.modulename, text, self.filename)
         data.setVarFlag(self.function, "func", 1)
         data.setVarFlag(self.function, "python", 1)
         data.setVar(self.function, text)
diff --git a/lib/bb/parse/parse_py/BBHandler.py b/lib/bb/parse/parse_py/BBHandler.py
index 81fb8d3..2aba9a0 100644
--- a/lib/bb/parse/parse_py/BBHandler.py
+++ b/lib/bb/parse/parse_py/BBHandler.py
@@ -166,10 +166,6 @@ def handle(fn, d, include):
     if oldfile:
         d.setVar("FILE", oldfile)
 
-    # we have parsed the bb class now
-    if ext == ".bbclass" or ext == ".inc":
-        bb.methodpool.set_parsed_module(base_name)
-
     return d
 
 def feeder(lineno, s, fn, root, statements):
-- 
1.8.3.2

Re: [1.18][PATCH] methodpool: Retire it, remove global method scope

[Richard Purdie]

On Tue, 2013-09-10 at 12:33 -0400, Denys Dmytriyenko wrote:
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> 
> Having a global method scope confuses users and with the introduction
> of parallel parsing, its not even possible to correctly detect conflicting
> functions. Rather than try and fix that, its simpler to retire the global
> method scope and restrict functions to those locations they're defined
> within. This is more what users actually expect too.
> 
> If we remove the global function scope, the need for methodpool is reduced
> to the point we may as well retire it. There is some small loss of caching
> of parsed functions but timing measurements so the impact to be neglibile
> in the overall parsing time.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> Tested-by: Denys Dmytriyenko <denys@ti.com>

In general I wouldn't take this (as a feature removal) however the code
in question was really broken when we introduced parallel parsing. Its
therefore totally broken, has been for a long time and this patch is
therefore a good thing as it is a bugfix rather than feature removal.

So I've merged it.

Cheers,

Richard

Re: [1.18][PATCH] methodpool: Retire it, remove global method scope

[Denys Dmytriyenko]

On Wed, Sep 11, 2013 at 06:00:05PM +0100, Richard Purdie wrote:
> On Tue, 2013-09-10 at 12:33 -0400, Denys Dmytriyenko wrote:
> > From: Richard Purdie <richard.purdie@linuxfoundation.org>
> > 
> > Having a global method scope confuses users and with the introduction
> > of parallel parsing, its not even possible to correctly detect conflicting
> > functions. Rather than try and fix that, its simpler to retire the global
> > method scope and restrict functions to those locations they're defined
> > within. This is more what users actually expect too.
> > 
> > If we remove the global function scope, the need for methodpool is reduced
> > to the point we may as well retire it. There is some small loss of caching
> > of parsed functions but timing measurements so the impact to be neglibile
> > in the overall parsing time.
> > 
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > Tested-by: Denys Dmytriyenko <denys@ti.com>
> 
> In general I wouldn't take this (as a feature removal) however the code
> in question was really broken when we introduced parallel parsing. Its
> therefore totally broken, has been for a long time and this patch is
> therefore a good thing as it is a bugfix rather than feature removal.
> 
> So I've merged it.

Thanks, Richard, appreciate it.

-- 
Denys