| Submitter | Richard Purdie |
|---|---|
| Date | May 9, 2012, 11:57 p.m. |
| Message ID | <1336607861.2494.113.camel@ted> |
| Download | mbox | patch |
| Permalink | /patch/27419/ |
| State | New |
| Headers | show |
Comments
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.
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
Patch
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):
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> ---