bitbake/runqueue: Fix 'full' stamp checking to be more efficient and cache results

bitbake/runqueue: Fix 'full' stamp checking to be more efficient and cache results

[Richard Purdie]

This fixes issues where bitbake would seemingly lock up when checking
certain configurations of stamp files due to deep recursion and
duplication.

This fixes a problem reported on the OE-Core mailing list to do with
BB_STAMP_POLICY = "full" appearing to hang upon rebuilds for long
periods of time (hours).

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py
index b870caf..48433be 100644
--- a/bitbake/lib/bb/runqueue.py
+++ b/bitbake/lib/bb/runqueue.py
@@ -875,7 +875,7 @@ class RunQueue:
             bb.msg.fatal("RunQueue", "check_stamps fatal internal error")
         return current
 
-    def check_stamp_task(self, task, taskname = None, recurse = False):
+    def check_stamp_task(self, task, taskname = None, recurse = False, cache = {}):
         def get_timestamp(f):
             try:
                 if not os.access(f, os.F_OK):
@@ -915,6 +915,9 @@ class RunQueue:
         t1 = get_timestamp(stampfile)
         for dep in self.rqdata.runq_depends[task]:
             if iscurrent:
+                if dep in cache:
+                    iscurrent = cache[dep]
+                    continue
                 fn2 = self.rqdata.taskData.fn_index[self.rqdata.runq_fnid[dep]]
                 taskname2 = self.rqdata.runq_task[dep]
                 stampfile2 = bb.build.stampfile(taskname2, self.rqdata.dataCache, fn2)
@@ -931,7 +934,9 @@ class RunQueue:
                         logger.debug(2, 'Stampfile %s < %s', stampfile, stampfile2)
                         iscurrent = False
                     if recurse and iscurrent:
-                        iscurrent = self.check_stamp_task(dep, recurse=True)
+                        iscurrent = self.check_stamp_task(dep, recurse=True, cache=cache)
+                        cache[dep] = iscurrent
+        cache[task] = iscurrent
         return iscurrent
 
     def execute_runqueue(self):

Re: bitbake/runqueue: Fix 'full' stamp checking to be more efficient and cache results

[Chris Larson]

On Wed, May 9, 2012 at 4:57 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py
> index b870caf..48433be 100644
> --- a/bitbake/lib/bb/runqueue.py
> +++ b/bitbake/lib/bb/runqueue.py
> @@ -875,7 +875,7 @@ class RunQueue:
>             bb.msg.fatal("RunQueue", "check_stamps fatal internal error")
>         return current
>
> -    def check_stamp_task(self, task, taskname = None, recurse = False):
> +    def check_stamp_task(self, task, taskname = None, recurse = False, cache = {}):

When people do this, it's typically a bug, but I presume you're doing
it intentionally here? Use of mutable default values is often
problematic due to their being shared across all calls to that
function, but that's okay for a cache. Maybe you intended this, given
it's a cache, but I wanted to ensure it was a conscious choice. Also,
this adds yet another global cache with no form of invalidation /
clear at all, it'll continue to grow through the lifetime of the
process.
-- 
Christopher Larson

Re: bitbake/runqueue: Fix 'full' stamp checking to be more efficient and cache results

[Richard Purdie]

On Wed, 2012-05-09 at 17:03 -0700, Chris Larson wrote:
> On Wed, May 9, 2012 at 4:57 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py
> > index b870caf..48433be 100644
> > --- a/bitbake/lib/bb/runqueue.py
> > +++ b/bitbake/lib/bb/runqueue.py
> > @@ -875,7 +875,7 @@ class RunQueue:
> >             bb.msg.fatal("RunQueue", "check_stamps fatal internal error")
> >         return current
> >
> > -    def check_stamp_task(self, task, taskname = None, recurse = False):
> > +    def check_stamp_task(self, task, taskname = None, recurse = False, cache = {}):
> 
> When people do this, it's typically a bug, but I presume you're doing
> it intentionally here? Use of mutable default values is often
> problematic due to their being shared across all calls to that
> function, but that's okay for a cache. Maybe you intended this, given
> it's a cache, but I wanted to ensure it was a conscious choice. Also,
> this adds yet another global cache with no form of invalidation /
> clear at all, it'll continue to grow through the lifetime of the
> process.

I'll change it to cache = None and then default it to {} in the code. I
agree infinitely growing caches in memory are not a good idea as we need
to avoid them as this could really screw up a UI doing re-execution.

Cheers,

Richard