[bitbake-devel] [PATCH 10/10] runqueue: report progress for "Preparing RunQueue" step

[Paul Eggleton]

On Mon, 13 Jun 2016 12:09:33 Richard Purdie wrote:
> On Mon, 2016-06-13 at 14:52 +1200, Paul Eggleton wrote:
> > @@ -1119,14 +1150,19 @@ class RunQueue:
> >          if self.state is runQueuePrepare:
> >              self.rqexe = RunQueueExecuteDummy(self)
> > 
> > +            self.rqdata.init_progress_reporter =
> > bb.progress.MultiStageProcessProgressReporter(self.cooker.data, +        
> >                                                    "Initialising tasks",
> > +                                                            [27, 870, 3,
> > 33, 1, 3, 3, 2, 3, 6, 1, 13, 1, 1, 132, 32, 1, 61, 1, 13, 2, 243, 151,
> > 299, 81, 1, 15, 72])> 
> >              if self.rqdata.prepare() == 0:
> >                  self.state = runQueueComplete
> >              
> >              else:
> >                  self.state = runQueueSceneInit
> > 
> > +                self.rqdata.init_progress_reporter.next_stage()
> 
> I appreciate the difficulty in figuring some of this out and the
> changes look quite neat on the most part. I am however very worried by
> the above from a code maintenance standpoint.
> 
> If for example I add or remove a step in runqueue, how do I know where
> in this array to make a change? How do I calculate what the new
> weighting should be?

Actually with the current patchset it's pretty easy. Just temporarily specify 
debug=True to the reporter when constructing and it will print out the list of 
weightings (based on timing each stage) when finish() is called, along with a 
map of the weightings to the lines where next_stage() was called. (Perhaps if 
we'd want a comment to that effect for anyone editing this code.)

> I did wonder if next_stage() should pass the number instead, that way
> at least the call site would be clearer. It doesn't solve the problem
> of knowing how many steps there are, or what these weights mean or how
> to recalculate them in the future.

I guess you could do it that way; in the way I'd structured the code earlier 
it wouldn't have been practical but now that next_stage() must be called 
before every stage, it could be done. The trouble though is that then if you 
want to set them all at once (as you often would, since they are relative) you 
will have to go to each line and update it instead of just changing the list 
passed to the constructor, so I'm not convinced that's an improvement.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre