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?
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: 1. keep reprogramming the same timer within the same phase 2. 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.