On 22-Nov 12:01, Vincent Guittot wrote:
On 22 November 2016 at 11:43, Patrick Bellasi patrick.bellasi@arm.com wrote:
Hi Vincent,
On 22-Nov 10:41, Vincent Guittot wrote:
On 21 November 2016 at 18:27, Juri Lelli juri.lelli@arm.com wrote:
On 21/11/16 16:13, Vincent Guittot wrote:
On 21 November 2016 at 15:43, Juri Lelli juri.lelli@arm.com wrote:
On 21/11/16 15:08, Vincent Guittot wrote: > Le Monday 21 Nov 2016 à 12:26:03 (+0000), Juri Lelli a écrit : > > On 21/11/16 10:33, Vincent Guittot wrote: > > > On 21 November 2016 at 10:28, Vincent Guittot > > > vincent.guittot@linaro.org wrote: > > > > On 17 November 2016 at 17:34, Juri Lelli juri.lelli@arm.com wrote:
[...]
> > Not sure I get where the problem is. Can you give me an example (and > > intended usage) of a phase with multiple timers? I don't seem to > > understand how that is supposed to work. :) > > Below is an example of a timer that is used twice in the same phase: > "tasks" : { > "thread0" : { > "instance" : 1, > "loop" : -1, > "run0" : 10000, > "timer0" : { "ref" : "timerA", "period" : 20000 }, > "run1" : 20000, > "timer1" : { "ref" : "timerA", "period" : 40000 }, > } > }, >
Mmm, but these are IMHO two distinct phases and we should describe them as such (maybe add a check to disallow such a json description). It
I don't think that we should disallow this kind of description. IMHO, it is more readable than splitting it in several phase
Then, what about this: "phase0" : { "loop" : 2, "run0" : 10000, "timer0" : { "ref" : "timerA", "period" : 20000 }, "run1" : 20000, "timer1" : { "ref" : "timerA", "period" : 40000 }, }, "phase1" : { "loop" : 1, "run0" : 20000, "timer0" : { "ref" : "timerA", "period" : 400000 }, "run1" : 10000, "timer1" : { "ref" : "timerA", "period" : 20000 }, }
I can probably come we other description involving different timers as well. Just to say that we should not restrict how to use events in a phase.
seems that we only make confusion descriptions possible (and we complicate handling from a code pow).
It's just about an addition instead of overwriting the value. That's already the behavior for duration, period and perf and that will probably be the case c_run as well
Not convinced yet. :)
IMHO, the following is more clear (while I agree that it is also more verbose :).
Not sure that it's more clear because it really depends of what you what to emulate. While trying to emulate a use case, you often apply phase to the different main steps of the use case. Then, I can probably come with more complex use cases that could not be easily unrolled.
In the example you provided before I cannot really see what we are trying to model. I understand that's a dummy example however it's hard to parse to me.
he first point seems like we want to define the time for the next activation of run0, this time is 20ms. Then we run immediately run1 and we reset the time to activate run0 after 40ms, completely overriding the previous configuration to start it every 20ms. Isn't that the expected semantic?
The example above is: start run 1 40ms after the start of run 0
I would say run1 starts 20ms after the start of run0, isn't it?
and start next event 20ms after the start of run1
mmm... not following here.
so you will not be impacted by the real duration of run0 and run1 (because of freq scaling or micro arch). Do this sequence 2 times then start the next sequence/phase
To me this is the expected behavior:
+-----+ +----------+ +-----+ +----------+ |r0 | |r1 | |r0 | |r1 | | | | | | | | | +-----------------------------------------------------------> 0 10 20 30 40 50 60 70 80 100 120 ^ ^ | | + + Timer0 Timer1
May be the confusion comes from how timer behave. Timer has been created as an extension of sleep. The main drawback of sleep is that it starts only when the previous event (like a run ) has finished whereas we sometime wants to start an even at a fixed point compared to the others whatever the duration of the previous run. So timer event has been created.
Which make sense.
But nothing prevents someone to use one or several timers one or several times on a phase.
I would say that timer is for sleep what runtime is for run. may be i should have name it fix_sleep instead of timer
Ok, let say we want to keep this flexibility than we have to agree on how to report the statistics. We usually reported stats per-phase... but perhaps if we allow multiple runs within a single phase than we should evaluate the possibility to report stats per each single run within a phase. For example, in the previous scenario it can be interesting to know if we are exceedin the 60ms timeout because of r0 or r1 being delayed.
If that's the case, IMO this semantic is quite confusing and it should be at least avoided or perhaps also forbidden. I'm not convinced that, in general, it can make sense at all to:
- keep reprogramming the same timer within the same phase
- execute two (or more) run commands back to back
Do we have more realistic use-cases where we found it useful having the flexibility you are proposing?
"tasks" : { "thread0" : { "instance" : 1, "loop" : 1, "delay" : 1000000, "cpus" : [1], "phases" : { "phase00" : { "loop" : 1, "run" : 10000, "timer" : { "ref" : "timerA", "period" : 20000 }, }, "phase01" : { "loop" : 1, "run" : 20000, "timer" : { "ref" : "timerA", "period" : 40000 }, }, "phase02" : { "loop" : 1, "run" : 10000, "timer" : { "ref" : "timerA", "period" : 20000 }, }, "phase03" : { "loop" : 1, "run" : 20000, "timer" : { "ref" : "timerA", "period" : 40000 }, }, "phase10" : { "loop" : 1, "run" : 20000, "timer" : { "ref" : "timerA", "period" : 400000 }, }, "phase11" : { "loop" : 1, "run" : 10000, "timer" : { "ref" : "timerA", "period" : 20000 }, } } } },
The difference is also that with your expression we get one row in the log for each phase (composed of two nested phases), and it seems that's already the behaviour without my modifications. With the latter approach
Yes i agree that it's the current approach and i want to keep the behavior possible as i don't want to put any constraint on how to use events in phases.
But if some constraints makes it more readable the workload description without compromising the possibility to model the real behaviors, where is the problem?
Do you think that the "extended form" does not allow to represent some use-casese?
Log timing is saved per phase so all metrics should be also set per phase whatever is put in this phase. This doesn't prevent someone to put only one event per phase in he wants to get metrics per event which seems to be what you want but IMO we should not prevent others to put several identical events in the same phase and still having metrics that reflect the content of the phase.
I agree on flexibility but provided it does not reduce readability of the described scenarios. I would like to be easy for a human to read and understand what will happen.
There also the c_run metrics in another patch, does it mean that you want only one run event per phase ?
This is something we need to extend and Juri has a proposal.
we instead get one entry for each different phase, which seems more inline with what one might expect, IMHO.
I think that both should be possible
I'm more for the "extended form" proposed by Juri, which put some constraints. Unless we convince ourself that is kind of limiting for the description of realistic use-cases which cannot be expressed in this form.
-- #include <best/regards.h>
Patrick Bellasi