Re: [Xenomai-core] measuring tasks execution time

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 4611 bytes --]

Daniel Simon wrote:
> On Fri, 08 Jun 2007 13:20:11 +0200
> Jan Kiszka <jan.kiszka@domain.hid> wrote:
> 
> Hello,
> 
>> Well, the best (==most comfortable :o) ) way from my POV would be if you
>> could try rebasing your work on your own first. Questions, even on minor
>> details, are always welcome, review on patches will be provided. Should
>> be no problem to get this into Xenomai 2.4.
>>
>> The rough to-do list would be:
>>
>>  o add some persistent runtimer counter to xnthread::stat and maintain
>>    it
>>  o introduce an xnpod service (inline function) to obtain it (let that
>>    thing return [0..LONGLONG_MAX] when stats are available, -1
>>    otherwise)
>>  o export the runtime via struct rt_task_info, maybe also the task start
>>    time
> 
> Coming back on this topic:
> please find attached a first draft of a patch following (more or less)  your
> suggestions:
> -there is an additional field "exectime" in  TASK_INFO, which can be set by
> rt_task_inquire and read in the info structure (the computation is slightly
> different for self and remote measurement);
> -xnthread_get_exectime() and xnthread_get_lastswitch() pass the values of the
> current thread's  timing out of xn;
> -exectime and lastswitch times are stored in an extended xnstat_runtime_t,
> updated by xnstat_runtime_update() (but not exactly like the "total" item)

That was not my idea, and there is a reason for it, see below.

> 
> it has been tested (only on  P3 single core cpu) with the testexec program also
> attached : there are 2 periodic tasks, clockit and loop, which measure
> either their exectime, or the one of the other task: the measures 
> seems reasonable, except when the clockit task measures itself, in that case the
> first and second printed values are always absurd for a reason I cannot
> understand...)

A few generic wishes related to your patch:

 - Please don't include debug or commented out code in your patches.

 - Please avoid unrelated changes

 - Please use an editor that properly formats your code, e.g. doesn't
   insert spaces where tabs belong according to kernel style

> 
> Only works in primary mode... (also I hope that the tsc is monotonic?)

Don't worry, TSC can be considered monotonic under Xenomai (that's why
you have to switch off frequency scaling e.g.).

> 
> I have no idea of what may happen on a smp or when on the fly migrating tasks!

SMP should not be an issue, given that we rely on more or less
synchronised TSCs anyway.

...
> diff -urN xenomai-2.3.1-orig/include/nucleus/stat.h xenomai-2.3.1/include/nucleus/stat.h
> --- xenomai-2.3.1-orig/include/nucleus/stat.h	2006-12-26 19:38:59.000000000 +0100
> +++ xenomai-2.3.1/include/nucleus/stat.h	2007-06-25 15:48:25.000000000 +0200
> @@ -31,6 +31,10 @@
>  
>  	xnticks_t total; /* Accumulated execution time */
>  
> +	xnticks_t exectime; /* Overall execution time from taskspawn upto lastswitch*/
> +
> +	xnticks_t lastswitch; /* Last time the task was made active */
> +
>  } xnstat_runtime_t;
>  
>  /* Return current date which can be passed to other xnstat services for
> @@ -42,6 +46,10 @@
>  do { \
>  	(sched)->current_account->total += \
>  		start - (sched)->last_account_switch; \
> +	(sched)->current_account->exectime += \
> +                xnstat_runtime_now() - (sched)->last_account_switch; \
> +	(sched)->current_account->lastswitch = \
> +                xnstat_runtime_now(); \

My idea was to keep a persistent version the existing xnstat_runtime_t
instance in xnthread (and later on also xnintr). That one shall not be
reset on readout via /proc.

Instead, you establish quite some new calculations that break
the existing API (the switch date is given via "start" - which was
misnamed so far, I just changed it to "date") and increase the runtime
overhead in the hotpath. Why? All the information you should need is
already there, it just has to be saved from being vaporised when the
user dumps /proc/xenomai/stat.

>  	(sched)->last_account_switch = start; \
>  } while (0)
>  

Let's try it like this: Change Xenomai so that it leaves the existing
xnthread_t::stat.account untouched when it reads /proc. Rather add
something like "xnstat_runtime_t last;" to xnthread_t::stat. On readout
for /proc output, do the stats now like "account-last" and then move
account into last. For your task exectime, you can then read
xnthread_t::stat.account directly, because it will always reflect the
full task history. Would't this work better?

Thanks for working on this!

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]