Re: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles from time API

2015-10-29 Thread Ivan Khoronzhuk 

Hi Petri,

Let's clarify some aspects that can be taken into account if common time type 
is used
for global and local times. Mainly it reflected in operation/conversion 
functions.
Mostly that is why I want to have separate functions for global and local times.
Please say what do you think about all of that.

1) odp_time_diff(t1, t2) behaves differently for local/global/interval 
timestamps

- if at least t1 is global time it must return timestamp of global time.
- if t1 and t2 are time intervals it returns time interval (in units local |
  global).
- if at least t1 is local time it must return timestamp of local time.

That can be implemented with hiding one additional var/flag under timestamp that
is redundancy and require to analyze it in every API call. Also it makes ns/time
conversion functions behave differently and returns one additional pseudo type
like interval, that is by a big account orthogonal to time type.

2) odp_time_cmp(odp_time_t t1, odp_time_t t2) behaves differently for
   local/global/interval time

- if comparing global time stamps, results gives what time was first
- you cannot compare local times as local time can wrap
- if comparing intervals, results gives what interval is bigger (for local |
  global).

3) uint64_t odp_time_to_ns(odp_time_t time) behaves differently for
   local/global/interval timestamps
- if converting global time, result is in "wall" ns.
- if converting local time, result is in local ticks as is.
- if converting local interval or local time, result is interval ns.
- if converting global interval, reult is in interval ns.

By a big account user knows what he is doing, so at least that is not a problem,
except redundancy.

4) odp_time_t odp_time_from_ns(uint64_t ns) creates set of functions
 - odp_time_local_from_ns()
 - odp_global_from_ns()
 - odp_global_interval_from_ns()
 - odp_local_interval_from_ns() // can be replaced by odp_time_local_from_ns()
// but better have for consistency.

5) odp_time_to_u64() also must return time/interval depending of the units of
   time type.

I'm sure that adding new time type can add even more redundancy in API, but
want to believe that local and global time are only times required, no see 
reason
to add smth else.

Also analyzing time type/interval/not interval in each function, several of 
which
them can be called in loops, is not very efficient. The same for global
timestamp for packets, it can be very frequently to call it stack, in which case
several saved CPU cycles for one call can save a thousands of cycles in
loops/stacks etc.

Adding global time as separate API can make life easier and reduce number of
API functions required to converting between different time types.


On 27.10.15 15:25, Savolainen, Petri (Nokia - FI/Espoo) wrote:

ODP time cannot be only local. Local time has limited use cases (measure < 4sec 
function call durations in time vs in CPU cycles). More interesting use cases 
demand global/sharable time. E.g. every packet may be time stamped in packet input 
(e.g. 1588 time stamp). Application is load balanced over multiple cores and those 
read/write/compare the packet time stamp along the processing pipeline. ODP time 
should be represented by one handle / type. Timer time (timer ticks) is not used 
for time calculation/comparisons, it's for controlling timers and timeout events.

C11 and DPDK uses struct timespec, which has sec + nsec fields. We'd need to 
minimize need for converting between  internal cycles and sec+nsec (performance 
penalty).

We can discuss this on the calls.

-Petri




-Original Message-
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Tuesday, October 27, 2015 2:22 PM
To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles
from time API

ping.

On 23.10.15 11:35, Ivan Khoronzhuk wrote:

Hi, Petri
Thanks for the reply.

On 23.10.15 10:57, Savolainen, Petri (Nokia - FI/Espoo) wrote:




-Original Message-
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Thursday, October 22, 2015 3:02 PM
To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo)
Cc: Ivan Khoronzhuk
Subject: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles
from time API

Current time API supposes that frequency of counter is equal
to CPU frequency. But that's not always true, for instance,
in case if no access to CPU cycle counter, another hi-resolution
timer can be used, and it`s rate can be different from CPU
rate. There is no big difference in which cycles to measure
time, the better hi-resolution timer the better measurements.
So, unbind CPU cycle counter from time API by eliminating word
"cycle" as it's believed to be used with CPU.

Also add new opaque type for time odp_time_t, as it asks user to use
API and abstracts time from units. New odp_time_t requires several
additional AP

Re: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles from time API

2015-10-27 Thread Savolainen, Petri (Nokia - FI/Espoo) 
ODP time cannot be only local. Local time has limited use cases (measure < 4sec 
function call durations in time vs in CPU cycles). More interesting use cases 
demand global/sharable time. E.g. every packet may be time stamped in packet 
input (e.g. 1588 time stamp). Application is load balanced over multiple cores 
and those read/write/compare the packet time stamp along the processing 
pipeline. ODP time should be represented by one handle / type. Timer time 
(timer ticks) is not used for time calculation/comparisons, it's for 
controlling timers and timeout events.

C11 and DPDK uses struct timespec, which has sec + nsec fields. We'd need to 
minimize need for converting between  internal cycles and sec+nsec (performance 
penalty).

We can discuss this on the calls.

-Petri



> -Original Message-
> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
> Sent: Tuesday, October 27, 2015 2:22 PM
> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles
> from time API
> 
> ping.
> 
> On 23.10.15 11:35, Ivan Khoronzhuk wrote:
> > Hi, Petri
> > Thanks for the reply.
> >
> > On 23.10.15 10:57, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> >>
> >>
> >>> -Original Message-
> >>> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
> >>> Sent: Thursday, October 22, 2015 3:02 PM
> >>> To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo)
> >>> Cc: Ivan Khoronzhuk
> >>> Subject: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles
> >>> from time API
> >>>
> >>> Current time API supposes that frequency of counter is equal
> >>> to CPU frequency. But that's not always true, for instance,
> >>> in case if no access to CPU cycle counter, another hi-resolution
> >>> timer can be used, and it`s rate can be different from CPU
> >>> rate. There is no big difference in which cycles to measure
> >>> time, the better hi-resolution timer the better measurements.
> >>> So, unbind CPU cycle counter from time API by eliminating word
> >>> "cycle" as it's believed to be used with CPU.
> >>>
> >>> Also add new opaque type for time odp_time_t, as it asks user to use
> >>> API and abstracts time from units. New odp_time_t requires several
> >>> additional API functions to be added:
> >>>
> >>> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2);
> >>> int odp_time_cmp(odp_time_t t1, odp_time_t t2);
> >>> uint64_t odp_time_to_u64(odp_time_t hdl);
> >>>
> >>> Also added new definition that represents 0 ticks for time -
> >>> ODP_TIME_NULL. It can be used instead of odp_time_from_ns(0) for
> >>> comparison and initialization.
> >>>
> >>> This patch changes only used time API, it doesn't change used var
> >>> names for simplicity.
> >>>
> >>> This time API can be implemented with local timer counter, so
> >>> shouldn't be used between threads.
> >>>
> >>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@linaro.org>
> >>> ---
> >>>   example/generator/odp_generator.c | 12 +++
> >>>   include/odp/api/time.h| 68
> ---
> >>> 
> >>>   test/performance/odp_pktio_perf.c | 49 +
> >>>   test/validation/pktio/pktio.c | 20 +--
> >>>   test/validation/scheduler/scheduler.c |  5 +--
> >>>   test/validation/time/time.c   | 27 +++---
> >>>   6 files changed, 114 insertions(+), 67 deletions(-)
> >>>
> >>> diff --git a/example/generator/odp_generator.c
> >>> b/example/generator/odp_generator.c
> >>> index be9597b..f84adc4 100644
> >>> --- a/example/generator/odp_generator.c
> >>> +++ b/example/generator/odp_generator.c
> >>> @@ -585,7 +585,7 @@ static void *gen_recv_thread(void *arg)
> >>>*/
> >>>   static void print_global_stats(int num_workers)
> >>>   {
> >>> -uint64_t start, wait, diff;
> >>> +odp_time_t start, wait, diff;
> >>>   uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
> >>>   int verbose_interval = 20;
> >>>   odp_thrmask_t thrd_mask;
> >>> @@ -593,8 +593,8 @@ static void print_global_stats(int num_workers)
> >>>   wh

Re: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles from time API

2015-10-27 Thread Ivan Khoronzhuk 

ping.

On 23.10.15 11:35, Ivan Khoronzhuk wrote:

Hi, Petri
Thanks for the reply.

On 23.10.15 10:57, Savolainen, Petri (Nokia - FI/Espoo) wrote:




-Original Message-
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Thursday, October 22, 2015 3:02 PM
To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo)
Cc: Ivan Khoronzhuk
Subject: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles
from time API

Current time API supposes that frequency of counter is equal
to CPU frequency. But that's not always true, for instance,
in case if no access to CPU cycle counter, another hi-resolution
timer can be used, and it`s rate can be different from CPU
rate. There is no big difference in which cycles to measure
time, the better hi-resolution timer the better measurements.
So, unbind CPU cycle counter from time API by eliminating word
"cycle" as it's believed to be used with CPU.

Also add new opaque type for time odp_time_t, as it asks user to use
API and abstracts time from units. New odp_time_t requires several
additional API functions to be added:

odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2);
int odp_time_cmp(odp_time_t t1, odp_time_t t2);
uint64_t odp_time_to_u64(odp_time_t hdl);

Also added new definition that represents 0 ticks for time -
ODP_TIME_NULL. It can be used instead of odp_time_from_ns(0) for
comparison and initialization.

This patch changes only used time API, it doesn't change used var
names for simplicity.

This time API can be implemented with local timer counter, so
shouldn't be used between threads.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@linaro.org>
---
  example/generator/odp_generator.c | 12 +++
  include/odp/api/time.h| 68 ---

  test/performance/odp_pktio_perf.c | 49 +
  test/validation/pktio/pktio.c | 20 +--
  test/validation/scheduler/scheduler.c |  5 +--
  test/validation/time/time.c   | 27 +++---
  6 files changed, 114 insertions(+), 67 deletions(-)

diff --git a/example/generator/odp_generator.c
b/example/generator/odp_generator.c
index be9597b..f84adc4 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -585,7 +585,7 @@ static void *gen_recv_thread(void *arg)
   */
  static void print_global_stats(int num_workers)
  {
-uint64_t start, wait, diff;
+odp_time_t start, wait, diff;
  uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
  int verbose_interval = 20;
  odp_thrmask_t thrd_mask;
@@ -593,8 +593,8 @@ static void print_global_stats(int num_workers)
  while (odp_thrmask_worker(_mask) < num_workers)
  continue;

-wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
-start = odp_time_cycles();
+wait = odp_time_from_ns(verbose_interval * ODP_TIME_SEC);
+start = odp_time();

  while (odp_thrmask_worker(_mask) == num_workers) {
  if (args->appl.number != -1 &&
@@ -603,11 +603,11 @@ static void print_global_stats(int num_workers)
  break;
  }

-diff = odp_time_diff_cycles(start, odp_time_cycles());
-if (diff < wait)
+diff = odp_time_diff(start, odp_time());
+if (odp_time_cmp(diff, wait) > 0)
  continue;

-start = odp_time_cycles();
+start = odp_time();

  if (args->appl.mode == APPL_MODE_RCV) {
  pkts = odp_atomic_load_u64();
diff --git a/include/odp/api/time.h b/include/odp/api/time.h
index b0072fc..7ed4734 100644
--- a/include/odp/api/time.h
+++ b/include/odp/api/time.h
@@ -28,14 +28,25 @@ extern "C" {
  #define ODP_TIME_MSEC 100ULL/**< Millisecond in nsec */
  #define ODP_TIME_SEC  10ULL /**< Second in nsec */



New name for these could be XXX_IN_NS
ODP_TIME_SEC_IN_NS  10ULL /**< Second in nsec */

Ok.
As was suggested, I will add it in separate patchset.






+/**
+ * @typedef odp_time_t
+ * ODP time stamp. Time stamp can be local, so shouldn't be shared between
+ * threads.
+ */



It's OK to implement local time first, but I'd define it so that global time is 
easy to add later.


I propose to split it by API, and don't use same function for that.
It allows to free casual time API from redundancy required by global time and 
different types of time.
Requirements for global time API:
- it must be 64-bit value, it's hard to imagine 32-bit source to compare with 
different threads.
- it must be wall time (in another case we cannot compare it)
- it never overflow (as it wall time) and if so, no need in _diff, _sum, _cmr 
functions, as we can do direct operations with 64-bit value
- it can require additional synchronization in implementation, so can be slower.
- we can get wall time as in "ticks", as in "ns" and directly compare it in 
appropriate units.

For local time it's not required to be wall

Re: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles from time API

2015-10-27 Thread Ivan Khoronzhuk 

On 27.10.15 15:25, Savolainen, Petri (Nokia - FI/Espoo) wrote:

ODP time cannot be only local. Local time has limited use cases (measure < 4sec 
function call durations in time vs in CPU cycles).

   > 4s, with 64-bit


 More interesting use cases demand global/sharable time. E.g.
 every packet may be time stamped in packet input (e.g. 1588 time stamp).
Application is load balanced over multiple cores and those read/write/compare 
the packet time stamp along the processing pipeline.

uint64_t odp_time_wall();
uint64_t odp_time_wall_ns();
?


 ODP time should be represented by one handle / type.

hiding type is time consuming operation for time catching functions.


 Timer time (timer ticks) is not used for time calculation/comparisons,
 it's for controlling timers and timeout events.

sure.



C11 and DPDK uses struct timespec, which has sec + nsec fields.
 We'd need to minimize need for converting between  internal cycles and 
sec+nsec (performance penalty).

sure. That's why conversion is made before loop.



We can discuss this on the calls.

-Petri




-Original Message-
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Tuesday, October 27, 2015 2:22 PM
To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles
from time API

ping.

On 23.10.15 11:35, Ivan Khoronzhuk wrote:

Hi, Petri
Thanks for the reply.

On 23.10.15 10:57, Savolainen, Petri (Nokia - FI/Espoo) wrote:




-Original Message-
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Thursday, October 22, 2015 3:02 PM
To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo)
Cc: Ivan Khoronzhuk
Subject: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles
from time API

Current time API supposes that frequency of counter is equal
to CPU frequency. But that's not always true, for instance,
in case if no access to CPU cycle counter, another hi-resolution
timer can be used, and it`s rate can be different from CPU
rate. There is no big difference in which cycles to measure
time, the better hi-resolution timer the better measurements.
So, unbind CPU cycle counter from time API by eliminating word
"cycle" as it's believed to be used with CPU.

Also add new opaque type for time odp_time_t, as it asks user to use
API and abstracts time from units. New odp_time_t requires several
additional API functions to be added:

odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2);
int odp_time_cmp(odp_time_t t1, odp_time_t t2);
uint64_t odp_time_to_u64(odp_time_t hdl);

Also added new definition that represents 0 ticks for time -
ODP_TIME_NULL. It can be used instead of odp_time_from_ns(0) for
comparison and initialization.

This patch changes only used time API, it doesn't change used var
names for simplicity.

This time API can be implemented with local timer counter, so
shouldn't be used between threads.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@linaro.org>
---
   example/generator/odp_generator.c | 12 +++
   include/odp/api/time.h| 68

---


   test/performance/odp_pktio_perf.c | 49 +
   test/validation/pktio/pktio.c | 20 +--
   test/validation/scheduler/scheduler.c |  5 +--
   test/validation/time/time.c   | 27 +++---
   6 files changed, 114 insertions(+), 67 deletions(-)

diff --git a/example/generator/odp_generator.c
b/example/generator/odp_generator.c
index be9597b..f84adc4 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -585,7 +585,7 @@ static void *gen_recv_thread(void *arg)
*/
   static void print_global_stats(int num_workers)
   {
-uint64_t start, wait, diff;
+odp_time_t start, wait, diff;
   uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
   int verbose_interval = 20;
   odp_thrmask_t thrd_mask;
@@ -593,8 +593,8 @@ static void print_global_stats(int num_workers)
   while (odp_thrmask_worker(_mask) < num_workers)
   continue;

-wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
-start = odp_time_cycles();
+wait = odp_time_from_ns(verbose_interval * ODP_TIME_SEC);
+start = odp_time();

   while (odp_thrmask_worker(_mask) == num_workers) {
   if (args->appl.number != -1 &&
@@ -603,11 +603,11 @@ static void print_global_stats(int num_workers)
   break;
   }

-diff = odp_time_diff_cycles(start, odp_time_cycles());
-if (diff < wait)
+diff = odp_time_diff(start, odp_time());
+if (odp_time_cmp(diff, wait) > 0)
   continue;

-start = odp_time_cycles();
+start = odp_time();

   if (args->appl.mode == APPL_MODE_RCV) {
   pkts = odp_atomic_load_u64();
diff --git a/include/odp/api/time.h b/include/odp/api/time.h
index b0072fc

Re: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles from time API

2015-10-23 Thread Savolainen, Petri (Nokia - FI/Espoo) 


> -Original Message-
> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
> Sent: Thursday, October 22, 2015 3:02 PM
> To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo)
> Cc: Ivan Khoronzhuk
> Subject: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles
> from time API
> 
> Current time API supposes that frequency of counter is equal
> to CPU frequency. But that's not always true, for instance,
> in case if no access to CPU cycle counter, another hi-resolution
> timer can be used, and it`s rate can be different from CPU
> rate. There is no big difference in which cycles to measure
> time, the better hi-resolution timer the better measurements.
> So, unbind CPU cycle counter from time API by eliminating word
> "cycle" as it's believed to be used with CPU.
> 
> Also add new opaque type for time odp_time_t, as it asks user to use
> API and abstracts time from units. New odp_time_t requires several
> additional API functions to be added:
> 
> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2);
> int odp_time_cmp(odp_time_t t1, odp_time_t t2);
> uint64_t odp_time_to_u64(odp_time_t hdl);
> 
> Also added new definition that represents 0 ticks for time -
> ODP_TIME_NULL. It can be used instead of odp_time_from_ns(0) for
> comparison and initialization.
> 
> This patch changes only used time API, it doesn't change used var
> names for simplicity.
> 
> This time API can be implemented with local timer counter, so
> shouldn't be used between threads.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@linaro.org>
> ---
>  example/generator/odp_generator.c | 12 +++
>  include/odp/api/time.h| 68 ---
> 
>  test/performance/odp_pktio_perf.c | 49 +
>  test/validation/pktio/pktio.c | 20 +--
>  test/validation/scheduler/scheduler.c |  5 +--
>  test/validation/time/time.c   | 27 +++---
>  6 files changed, 114 insertions(+), 67 deletions(-)
> 
> diff --git a/example/generator/odp_generator.c
> b/example/generator/odp_generator.c
> index be9597b..f84adc4 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -585,7 +585,7 @@ static void *gen_recv_thread(void *arg)
>   */
>  static void print_global_stats(int num_workers)
>  {
> - uint64_t start, wait, diff;
> + odp_time_t start, wait, diff;
>   uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
>   int verbose_interval = 20;
>   odp_thrmask_t thrd_mask;
> @@ -593,8 +593,8 @@ static void print_global_stats(int num_workers)
>   while (odp_thrmask_worker(_mask) < num_workers)
>   continue;
> 
> - wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
> - start = odp_time_cycles();
> + wait = odp_time_from_ns(verbose_interval * ODP_TIME_SEC);
> + start = odp_time();
> 
>   while (odp_thrmask_worker(_mask) == num_workers) {
>   if (args->appl.number != -1 &&
> @@ -603,11 +603,11 @@ static void print_global_stats(int num_workers)
>   break;
>   }
> 
> - diff = odp_time_diff_cycles(start, odp_time_cycles());
> - if (diff < wait)
> + diff = odp_time_diff(start, odp_time());
> + if (odp_time_cmp(diff, wait) > 0)
>   continue;
> 
> - start = odp_time_cycles();
> + start = odp_time();
> 
>   if (args->appl.mode == APPL_MODE_RCV) {
>   pkts = odp_atomic_load_u64();
> diff --git a/include/odp/api/time.h b/include/odp/api/time.h
> index b0072fc..7ed4734 100644
> --- a/include/odp/api/time.h
> +++ b/include/odp/api/time.h
> @@ -28,14 +28,25 @@ extern "C" {
>  #define ODP_TIME_MSEC 100ULL/**< Millisecond in nsec */
>  #define ODP_TIME_SEC  10ULL /**< Second in nsec */


New name for these could be XXX_IN_NS
ODP_TIME_SEC_IN_NS  10ULL /**< Second in nsec */


> 
> +/**
> + * @typedef odp_time_t
> + * ODP time stamp. Time stamp can be local, so shouldn't be shared between
> + * threads.
> + */


It's OK to implement local time first, but I'd define it so that global time is 
easy to add later.

/**
 * @typedef odp_time_t
 * ODP time stamp. Time stamp can be represent a time stamp from local or 
global time source.
 * A local time stamp must not be shared between threads. API calls work 
correctly only when
 * all time stamps for input are from the same time source.
 */

In case of 64 bit time stamps, implementation can either define odp_time_t as a 
small struct or use couple of MSB bits

Re: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles from time API

2015-10-23 Thread Ivan Khoronzhuk 

Hi, Petri
Thanks for the reply.

On 23.10.15 10:57, Savolainen, Petri (Nokia - FI/Espoo) wrote:




-Original Message-
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Thursday, October 22, 2015 3:02 PM
To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo)
Cc: Ivan Khoronzhuk
Subject: [lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles
from time API

Current time API supposes that frequency of counter is equal
to CPU frequency. But that's not always true, for instance,
in case if no access to CPU cycle counter, another hi-resolution
timer can be used, and it`s rate can be different from CPU
rate. There is no big difference in which cycles to measure
time, the better hi-resolution timer the better measurements.
So, unbind CPU cycle counter from time API by eliminating word
"cycle" as it's believed to be used with CPU.

Also add new opaque type for time odp_time_t, as it asks user to use
API and abstracts time from units. New odp_time_t requires several
additional API functions to be added:

odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2);
int odp_time_cmp(odp_time_t t1, odp_time_t t2);
uint64_t odp_time_to_u64(odp_time_t hdl);

Also added new definition that represents 0 ticks for time -
ODP_TIME_NULL. It can be used instead of odp_time_from_ns(0) for
comparison and initialization.

This patch changes only used time API, it doesn't change used var
names for simplicity.

This time API can be implemented with local timer counter, so
shouldn't be used between threads.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@linaro.org>
---
  example/generator/odp_generator.c | 12 +++
  include/odp/api/time.h| 68 ---

  test/performance/odp_pktio_perf.c | 49 +
  test/validation/pktio/pktio.c | 20 +--
  test/validation/scheduler/scheduler.c |  5 +--
  test/validation/time/time.c   | 27 +++---
  6 files changed, 114 insertions(+), 67 deletions(-)

diff --git a/example/generator/odp_generator.c
b/example/generator/odp_generator.c
index be9597b..f84adc4 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -585,7 +585,7 @@ static void *gen_recv_thread(void *arg)
   */
  static void print_global_stats(int num_workers)
  {
-   uint64_t start, wait, diff;
+   odp_time_t start, wait, diff;
uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
int verbose_interval = 20;
odp_thrmask_t thrd_mask;
@@ -593,8 +593,8 @@ static void print_global_stats(int num_workers)
while (odp_thrmask_worker(_mask) < num_workers)
continue;

-   wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
-   start = odp_time_cycles();
+   wait = odp_time_from_ns(verbose_interval * ODP_TIME_SEC);
+   start = odp_time();

while (odp_thrmask_worker(_mask) == num_workers) {
if (args->appl.number != -1 &&
@@ -603,11 +603,11 @@ static void print_global_stats(int num_workers)
break;
}

-   diff = odp_time_diff_cycles(start, odp_time_cycles());
-   if (diff < wait)
+   diff = odp_time_diff(start, odp_time());
+   if (odp_time_cmp(diff, wait) > 0)
continue;

-   start = odp_time_cycles();
+   start = odp_time();

if (args->appl.mode == APPL_MODE_RCV) {
pkts = odp_atomic_load_u64();
diff --git a/include/odp/api/time.h b/include/odp/api/time.h
index b0072fc..7ed4734 100644
--- a/include/odp/api/time.h
+++ b/include/odp/api/time.h
@@ -28,14 +28,25 @@ extern "C" {
  #define ODP_TIME_MSEC 100ULL/**< Millisecond in nsec */
  #define ODP_TIME_SEC  10ULL /**< Second in nsec */



New name for these could be XXX_IN_NS
ODP_TIME_SEC_IN_NS  10ULL /**< Second in nsec */

Ok.
As was suggested, I will add it in separate patchset.






+/**
+ * @typedef odp_time_t
+ * ODP time stamp. Time stamp can be local, so shouldn't be shared between
+ * threads.
+ */



It's OK to implement local time first, but I'd define it so that global time is 
easy to add later.


I propose to split it by API, and don't use same function for that.
It allows to free casual time API from redundancy required by global time and 
different types of time.
Requirements for global time API:
- it must be 64-bit value, it's hard to imagine 32-bit source to compare with 
different threads.
- it must be wall time (in another case we cannot compare it)
- it never overflow (as it wall time) and if so, no need in _diff, _sum, _cmr 
functions, as we can do direct operations with 64-bit value
- it can require additional synchronization in implementation, so can be slower.
- we can get wall time as in "ticks", as in "ns" and directly

[lng-odp] [API-NEXT PATCH v5 3/5] api: time: unbind CPU cycles from time API

2015-10-22 Thread Ivan Khoronzhuk 
Current time API supposes that frequency of counter is equal
to CPU frequency. But that's not always true, for instance,
in case if no access to CPU cycle counter, another hi-resolution
timer can be used, and it`s rate can be different from CPU
rate. There is no big difference in which cycles to measure
time, the better hi-resolution timer the better measurements.
So, unbind CPU cycle counter from time API by eliminating word
"cycle" as it's believed to be used with CPU.

Also add new opaque type for time odp_time_t, as it asks user to use
API and abstracts time from units. New odp_time_t requires several
additional API functions to be added:

odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2);
int odp_time_cmp(odp_time_t t1, odp_time_t t2);
uint64_t odp_time_to_u64(odp_time_t hdl);

Also added new definition that represents 0 ticks for time -
ODP_TIME_NULL. It can be used instead of odp_time_from_ns(0) for
comparison and initialization.

This patch changes only used time API, it doesn't change used var
names for simplicity.

This time API can be implemented with local timer counter, so
shouldn't be used between threads.

Signed-off-by: Ivan Khoronzhuk 
---
 example/generator/odp_generator.c | 12 +++
 include/odp/api/time.h| 68 ---
 test/performance/odp_pktio_perf.c | 49 +
 test/validation/pktio/pktio.c | 20 +--
 test/validation/scheduler/scheduler.c |  5 +--
 test/validation/time/time.c   | 27 +++---
 6 files changed, 114 insertions(+), 67 deletions(-)

diff --git a/example/generator/odp_generator.c 
b/example/generator/odp_generator.c
index be9597b..f84adc4 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -585,7 +585,7 @@ static void *gen_recv_thread(void *arg)
  */
 static void print_global_stats(int num_workers)
 {
-   uint64_t start, wait, diff;
+   odp_time_t start, wait, diff;
uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
int verbose_interval = 20;
odp_thrmask_t thrd_mask;
@@ -593,8 +593,8 @@ static void print_global_stats(int num_workers)
while (odp_thrmask_worker(_mask) < num_workers)
continue;
 
-   wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
-   start = odp_time_cycles();
+   wait = odp_time_from_ns(verbose_interval * ODP_TIME_SEC);
+   start = odp_time();
 
while (odp_thrmask_worker(_mask) == num_workers) {
if (args->appl.number != -1 &&
@@ -603,11 +603,11 @@ static void print_global_stats(int num_workers)
break;
}
 
-   diff = odp_time_diff_cycles(start, odp_time_cycles());
-   if (diff < wait)
+   diff = odp_time_diff(start, odp_time());
+   if (odp_time_cmp(diff, wait) > 0)
continue;
 
-   start = odp_time_cycles();
+   start = odp_time();
 
if (args->appl.mode == APPL_MODE_RCV) {
pkts = odp_atomic_load_u64();
diff --git a/include/odp/api/time.h b/include/odp/api/time.h
index b0072fc..7ed4734 100644
--- a/include/odp/api/time.h
+++ b/include/odp/api/time.h
@@ -28,14 +28,25 @@ extern "C" {
 #define ODP_TIME_MSEC 100ULL/**< Millisecond in nsec */
 #define ODP_TIME_SEC  10ULL /**< Second in nsec */
 
+/**
+ * @typedef odp_time_t
+ * ODP time stamp. Time stamp can be local, so shouldn't be shared between
+ * threads.
+ */
 
 /**
- * Current time in CPU cycles
- *
- * @return Current time in CPU cycles
+ * @def ODP_TIME_NULL
+ * Zero time stamp
  */
-uint64_t odp_time_cycles(void);
 
+/**
+ * Current time stamp.
+ *
+ * It should be hi-resolution time.
+ *
+ * @return Time stamp.
+ */
+odp_time_t odp_time(void);
 
 /**
  * Time difference
@@ -43,29 +54,60 @@ uint64_t odp_time_cycles(void);
  * @param t1First time stamp
  * @param t2Second time stamp
  *
- * @return Difference of time stamps in CPU cycles
+ * @return Difference of time stamps
  */
-uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2);
+odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2);
 
+/**
+ * Time sum
+ *
+ * @param t1Time stamp
+ * @param t2Time stamp
+ *
+ * @return Sum of time stamps
+ */
+odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2);
 
 /**
- * Convert CPU cycles to nanoseconds
+ * Convert time to nanoseconds
  *
- * @param cycles  Time in CPU cycles
+ * @param time  Time
  *
  * @return Time in nanoseconds
  */
-uint64_t odp_time_cycles_to_ns(uint64_t cycles);
-
+uint64_t odp_time_to_ns(odp_time_t time);
 
 /**
- * Convert nanoseconds to CPU cycles
+ * Convert nanoseconds to time
  *
  * @param ns  Time in nanoseconds
  *
- * @return Time in CPU cycles
+ * @return Time stamp
+ */
+odp_time_t odp_time_from_ns(uint64_t ns);
+
+/**
+ * Compare two times as absolute ranges
+ *
+ * @param t1