Skip to contentSkip to conversation

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

PATCH Richard Purdie 2 participants 3 messages May 10, 2012

lore
Share & tools
lore
#0 lore raw

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).

---

bitbake/lib/bb/runqueue.py +7 -2
index b870caf..48433be 100644
@@ -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):
#1 lore raw
Quoted 10 lines · Richard Purdie
bitbake/lib/bb/runqueue.py +0 -0
index b870caf..48433be 100644
@@ -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.

Signature · 1 line
Christopher Larson
#2 lore raw
Quoted 10 lines · Chris Larson
Quoted 8 lines · Richard Purdie
bitbake/lib/bb/runqueue.py +0 -0
index b870caf..48433be 100644
@@ -875,7 +875,7 @@ class RunQueue:
            bb.msg.fatal("RunQueue", "check_stamps fatal internal error")
        return current
 
Quoted 11 lines

bitbake/lib/bb/runqueue.py class RunQueue: · line 878

Quoted 2 lines
bitbake/lib/bb/runqueue.py +1 -1
-    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