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

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

[Richard Purdie]

This should fix issues where bitbake would seemingly lock up when checking
certain configurations of stampfiles.

The cache is kept within the runqueue since that feels like the right
place to associate this cache data.

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..0eed9ee 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 = None):
         def get_timestamp(f):
             try:
                 if not os.access(f, os.F_OK):
@@ -911,10 +911,16 @@ class RunQueue:
         if taskname != "do_setscene" and taskname.endswith("_setscene"):
             return True
 
+        if cache is None:
+            cache = {}
+
         iscurrent = True
         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 +937,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):
@@ -1041,6 +1049,8 @@ class RunQueueExecute:
         self.build_stamps = {}
         self.failed_fnids = []
 
+        self.stampcache = {}
+
     def runqueue_process_waitpid(self):
         """
         Return none is there are no processes awaiting result collection, otherwise
@@ -1373,7 +1383,7 @@ class RunQueueExecuteTasks(RunQueueExecute):
                 self.task_skip(task)
                 return True
 
-            if self.rq.check_stamp_task(task, taskname):
+            if self.rq.check_stamp_task(task, taskname, cache=self.stampcache):
                 logger.debug(2, "Stamp current task %s (%s)", task,
                                 self.rqdata.get_user_idstring(task))
                 self.task_skip(task)
@@ -1557,7 +1567,7 @@ class RunQueueExecuteScenequeue(RunQueueExecute):
                     bb.build.make_stamp(taskname + "_setscene", self.rqdata.dataCache, fn)
                     continue
 
-                if self.rq.check_stamp_task(realtask, taskname + "_setscene"):
+                if self.rq.check_stamp_task(realtask, taskname + "_setscene", cache=self.stampcache):
                     logger.debug(2, 'Setscene stamp current for task %s(%s)', task, self.rqdata.get_user_idstring(realtask))
                     stamppresent.append(task)
                     self.task_skip(task)
@@ -1650,7 +1660,7 @@ class RunQueueExecuteScenequeue(RunQueueExecute):
             fn = self.rqdata.taskData.fn_index[self.rqdata.runq_fnid[realtask]]
 
             taskname = self.rqdata.runq_task[realtask] + "_setscene"
-            if self.rq.check_stamp_task(realtask, self.rqdata.runq_task[realtask], recurse = True):
+            if self.rq.check_stamp_task(realtask, self.rqdata.runq_task[realtask], recurse = True, cache=self.stampcache):
                 logger.debug(2, 'Stamp for underlying task %s(%s) is current, so skipping setscene variant',
                              task, self.rqdata.get_user_idstring(realtask))
                 self.task_failoutright(task)
@@ -1662,7 +1672,7 @@ class RunQueueExecuteScenequeue(RunQueueExecute):
                         self.task_failoutright(task)
                         return True
 
-            if self.rq.check_stamp_task(realtask, taskname):
+            if self.rq.check_stamp_task(realtask, taskname, cache=self.stampcache):
                 logger.debug(2, 'Setscene stamp current task %s(%s), so skip it and its dependencies',
                              task, self.rqdata.get_user_idstring(realtask))
                 self.task_skip(task)

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

[Chris Larson]

On Thu, May 10, 2012 at 1:21 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> This should fix issues where bitbake would seemingly lock up when checking
> certain configurations of stampfiles.
>
> The cache is kept within the runqueue since that feels like the right
> place to associate this cache data.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

Looks good to me.
-- 
Christopher Larson