Message ID | 20231214134528.1973602-3-alex@linutronix.de |
---|---|
State | New |
Headers | show |
Series | [1/9] oeqa/selftest/sstatetests: re-work CDN tests, add local cache tests | expand |
On Thu, 2023-12-14 at 14:45 +0100, Alexander Kanavin wrote: > printdiff needs setscene dependencies and they're available only > through that object. Previously it was instantianted just after > running printdiff, this moves the initilization to just prior. > > Signed-off-by: Alexander Kanavin <alex@linutronix.de> > --- > bitbake/lib/bb/runqueue.py | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py > index 864708ee4a5..1f59d18b71a 100644 > --- a/bitbake/lib/bb/runqueue.py > +++ b/bitbake/lib/bb/runqueue.py > @@ -1555,6 +1555,11 @@ class RunQueue: > ('bb.event.HeartbeatEvent',), data=self.cfgData) > self.dm_event_handler_registered = True > > + self.rqdata.init_progress_reporter.next_stage() > + self.start_worker() > + self.rqdata.init_progress_reporter.next_stage() > + self.rqexe = RunQueueExecute(self) > + > dump = self.cooker.configuration.dump_signatures > if dump: > self.rqdata.init_progress_reporter.finish() > @@ -1566,11 +1571,6 @@ class RunQueue: > self.state = runQueueComplete > > if self.state is runQueueSceneInit: > - self.rqdata.init_progress_reporter.next_stage() > - self.start_worker() > - self.rqdata.init_progress_reporter.next_stage() > - self.rqexe = RunQueueExecute(self) > - > # If we don't have any setscene functions, skip execution > if not self.rqdata.runq_setscene_tids: > logger.info('No setscene tasks') I not entirely happy about this since start_worker() executes processes and isn't trivial. The code is careful enough to tear them down too at exit but it is all a bit of a waste of time. I had wondered if we can create RunQueueExecute() without the workers but as the code stands, it does poke things into them in a small isolated section. I think this code flow should be tweaked to stop RunQueueExecute needing the workers to be started and that would be a decent cleanup of the code anyway. I can take a look at that if it helps since I think I've been moving towards that refactor for a while anyway? Cheers, Richard
On Thu, 14 Dec 2023 at 15:39, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > I not entirely happy about this since start_worker() executes processes > and isn't trivial. The code is careful enough to tear them down too at > exit but it is all a bit of a waste of time. > > I had wondered if we can create RunQueueExecute() without the workers > but as the code stands, it does poke things into them in a small > isolated section. I think this code flow should be tweaked to stop > RunQueueExecute needing the workers to be started and that would be a > decent cleanup of the code anyway. > > I can take a look at that if it helps since I think I've been moving > towards that refactor for a while anyway? Yes please. I haven't noticed any regression in printdiff performance, other than a couple of additional lines printed, but if you can make a better patch, that'd be welcome. Meanwhile I'd like to implement what I mentioned in note (1) in patch 4/9, as it's a real regression that I realized only today as I was preparing the patchset for submission and now it's bothering me :) Alex
On Thu, 2023-12-14 at 18:28 +0100, Alexander Kanavin wrote: > On Thu, 14 Dec 2023 at 15:39, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > I not entirely happy about this since start_worker() executes processes > > and isn't trivial. The code is careful enough to tear them down too at > > exit but it is all a bit of a waste of time. > > > > I had wondered if we can create RunQueueExecute() without the workers > > but as the code stands, it does poke things into them in a small > > isolated section. I think this code flow should be tweaked to stop > > RunQueueExecute needing the workers to be started and that would be a > > decent cleanup of the code anyway. > > > > I can take a look at that if it helps since I think I've been moving > > towards that refactor for a while anyway? > > Yes please. I haven't noticed any regression in printdiff performance, > other than a couple of additional lines printed, but if you can make a > better patch, that'd be welcome. Patch on the bitbake list for this which removes more code than it adds. We had already set everything up to do this :) > Meanwhile I'd like to implement what I mentioned in note (1) in patch > 4/9, as it's a real regression that I realized only today as I was > preparing the patchset for submission and now it's bothering me :) Yes, it is and I do agree we need to fix that. Cheers, Richard
On Fri, 15 Dec 2023 at 17:04, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > Meanwhile I'd like to implement what I mentioned in note (1) in patch > > 4/9, as it's a real regression that I realized only today as I was > > preparing the patchset for submission and now it's bothering me :) > > Yes, it is and I do agree we need to fix that. I just completed the fixing. It all works wonderfully in local builds, but who knows what situations AB will be able to come up with :) It's a bit crowded on the AB right now to start the tests there, so here are the patches: https://git.yoctoproject.org/poky-contrib/log/?h=akanavin/fix-printdiff (I run the tests with a special contrib/kanavin/ branch of autobuilder-helper that only runs the three needed selftests, and not the whole gigantic set) I also reverted the gnu-config tweak in that branch as it should be no longer necessary. I'll cherry-pick the bitbake patch before doing any further tests, but it's now Friday evening, so anxiously watching ongoing selftests is perhaps not the best idea. Alex
On Fri, 2023-12-15 at 17:49 +0100, Alexander Kanavin wrote: > On Fri, 15 Dec 2023 at 17:04, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > > Meanwhile I'd like to implement what I mentioned in note (1) in patch > > > 4/9, as it's a real regression that I realized only today as I was > > > preparing the patchset for submission and now it's bothering me :) > > > > Yes, it is and I do agree we need to fix that. > > I just completed the fixing. It all works wonderfully in local builds, > but who knows what situations AB will be able to come up with :) It's > a bit crowded on the AB right now to start the tests there, so here > are the patches: > https://git.yoctoproject.org/poky-contrib/log/?h=akanavin/fix-printdiff > (I run the tests with a special contrib/kanavin/ branch of > autobuilder-helper that only runs the three needed selftests, and not > the whole gigantic set) Sounds good. I think we are getting to the bottom of some of the underlying issues which is good and longer term should help usability a lot. > I also reverted the gnu-config tweak in that branch as it should be no > longer necessary. I was torn on that. I can drop from -next too. > I'll cherry-pick the bitbake patch before doing any further tests, but > it's now Friday evening, so anxiously watching ongoing selftests is > perhaps not the best idea. Fair enough! I've run that patch through local only testing so far, as you say, the AB has a lot going on. I've been torn on waiting for these or building M1 but I'm aiming for just the CDN fix since this may or may not take a while to settle on the autobuilder and I don't think we need any more pressure. The failures are driving Alexandre a bit crazy in swatbot so getting things sorted is important. Cheers, Richard
diff --git a/bitbake/lib/bb/runqueue.py b/bitbake/lib/bb/runqueue.py index 864708ee4a5..1f59d18b71a 100644 --- a/bitbake/lib/bb/runqueue.py +++ b/bitbake/lib/bb/runqueue.py @@ -1555,6 +1555,11 @@ class RunQueue: ('bb.event.HeartbeatEvent',), data=self.cfgData) self.dm_event_handler_registered = True + self.rqdata.init_progress_reporter.next_stage() + self.start_worker() + self.rqdata.init_progress_reporter.next_stage() + self.rqexe = RunQueueExecute(self) + dump = self.cooker.configuration.dump_signatures if dump: self.rqdata.init_progress_reporter.finish() @@ -1566,11 +1571,6 @@ class RunQueue: self.state = runQueueComplete if self.state is runQueueSceneInit: - self.rqdata.init_progress_reporter.next_stage() - self.start_worker() - self.rqdata.init_progress_reporter.next_stage() - self.rqexe = RunQueueExecute(self) - # If we don't have any setscene functions, skip execution if not self.rqdata.runq_setscene_tids: logger.info('No setscene tasks')
printdiff needs setscene dependencies and they're available only through that object. Previously it was instantianted just after running printdiff, this moves the initilization to just prior. Signed-off-by: Alexander Kanavin <alex@linutronix.de> --- bitbake/lib/bb/runqueue.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)