On 09/16/16 12:51, Savolainen, Petri (Nokia - FI/Espoo) wrote:


-----Original Message-----
From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
Sent: Friday, September 16, 2016 10:05 AM
To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-
labs.com>; lng-odp-forward <lng-odp@lists.linaro.org>
Subject: Re: [lng-odp] [API-NEXT PATCHv2] api: odp_time_sub

On 09/16/16 09:41, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
Maxim
Uvarov
Sent: Thursday, September 15, 2016 7:07 PM
To: lng-odp-forward <lng-odp@lists.linaro.org>
Subject: Re: [lng-odp] [API-NEXT PATCHv2] api: odp_time_sub

On 09/15/16 15:18, Maxim Uvarov wrote:
there is odp_time_diff(), forget about this patch.

Sorry, odp_time_diff() is for difference. odp_time_sub() still needed
for thing like calculate interval for execution.

Please review api patch.

Maxim.

Could you give an example. E.g. this kind of interval calculation does
not need substract:
t1 = time();
t2 = t1 + wait_time;

while (time() < t2)
      spin();

// now it's time to continue
foo();


-Petri
Looks like I lost again. "diff" word is confusing for me:
/**
   * Time difference
   *
   * @param t2    Second time stamp
   * @param t1    First time stamp
   *
   * @return Difference of time stamps
   */
odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1);


In fact this function (at least in linux-generic) subtracts t1 from t2.
Assuming that t2 > t1.

I'm going to send proposal api patch to implement function:
int odp_schedule_stats(odp_schedule_stats_t *stats);
which will return times spend in and out scheduler. Later extend it
with more counters like context switches and etc.

t1 = odp_time_local();
ev = sched_api->schedule(from, wait);
t2 = odp_time_local();

Then time in schedule will be:
odp_time_t sched_time =  odp_time_diff(t2, t1)

So I think everything ok but I would refine doxygen to be more clear
what is odp_time_diff() for
and probably rename it to odp_time_sub() to be more symmetrical to
odp_time_sum().

Maxim.


Diff is commonly used for time difference where only t2 - t1 is allowed (not t1 
- t2). Sub is more generic term and would indicate that also t1 - t2 is 
allowed, which would produce a *negative* result. We'd not want to support 
negative time. Also odp_time_diff is similar to e.g. POSIX difftime( time_t 
time_end, time_t time_beg )
ok.
About scheduler statistics. Features that are commonly HW accelerated fit into 
the API, while those that are not commonly supported should not be added. When 
e.g. scheduler execution time statistics is not commonly calculated by the HW, 
it's better to leave it to the (interested) application. Additional statistics 
add implementation complexity and decrease performance. Time read is bad for 
performance since it requires synchronization (system call, read a SoC global 
register, etc) and cannot be reordered.

Application can easily do:

t1 = odp_time_local();
ev = odp_schedule(from, wait);
t2 = odp_time_local();
        
there's no room for implementation to do it any faster.


-Petri

Yes, anyway there will be slow down. I'm looking to card ODP-332 <https://projects.linaro.org/browse/ODP-332>. Time spend for app is odp_time_local() - sched_time. And sched time calculated as code above. Time spend can be calculated with cpu cycles or performance counters on cpu. But any accounting will take time for such critical function. And probably there will be no common values which we can get from hardware or from OS. Maybe it's better to close that card.

Maxim.



Reply via email to