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:[email protected]]
Sent: Thursday, October 22, 2015 3:02 PM
To: [email protected]; 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 <[email protected]>
---
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(&thrd_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(&thrd_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(&counters.udp);
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 1000000ULL /**< Millisecond in nsec */
#define ODP_TIME_SEC 1000000000ULL /**< Second in nsec */
New name for these could be XXX_IN_NS
ODP_TIME_SEC_IN_NS 1000000000ULL /**< 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 time, as so wall time is always
global.
I think it's better to leave current API as local by default, and use additional
wall time several calls as global (it's never wrap in practice)
Like:
uint64_t odp_time_wall();
uint64_t odp_time_wall_ns();
and conversion functions for them:
odp_time_wall_from_ns(); // don't take into account any start time and overlap,
simply convert ranges
odp_time_wall_to_ns(); // don't take into account any start time and overlap,
simply convert ranges
With these calls no need to change time odp_time() to odp_time_local().
It global time needed (and it must be wall) application uses wall time calls.
/**
* @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.
I thinks that's enough.
API calls work correctly only when all time stamps for input are from the same
time source.
It's redundancy.
*/
In case of 64 bit time stamps, implementation can either define odp_time_t as a
small struct or use couple of MSB bits for flags (e.g. time stamp would wrap in
290 years instead of 580 years)
With separate calls for wall time it's not needed.
/**
- * 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);
This is then renamed to be the local time source
I think no need to overload API with time type here.
It's obvious that it cannot be used as global in case if we have wall time API
and correctly document it.
/**
* Current local time
*
* Returns current thread local time stamp value. The local time
* source provides high resolution but may wrap between two time
* stamp calls. Local time should be used only for short time
* range calculations or comparisons.
*
* @return Local time stamp
*/
odp_time_t odp_time_local(void);
We can add later a call to query the local time stamp wrap around time (max
range).
We can, but what practical usage for that.
Application can use it at init only for check if time source is appropriate for
that.
But more correct way is to write correct application and for huge time ranges
use timers.
And I'm mostly sure that 32-bit counter will not be used for that, but anyway,
why not to add it in the future.
All other calls can remain as is.
-Petri
/**
* Time difference
@@ -43,29 +54,60 @@ uint64_t odp_time_cycles(void);
* @param t1 First time stamp
* @param t2 Second 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 t1 Time stamp
+ * @param t2 Time 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 First time
+ * @param t2 Second time
+ *
+ * @retval -1 if t2 < t1, 0 if t1 = t2, 1 if t2 > t1
+ */
+int odp_time_cmp(odp_time_t t1, odp_time_t t2);
+
+/**
+ * Get printable value for an odp_time_t
+ *
+ * @param time time to be printed
+ * @return uint64_t value that can be used to print/display this
+ * time
+ *
+ * @note This routine is intended to be used for diagnostic purposes
+ * to enable applications to generate a printable value that represents
+ * an odp_time_t time.
*/
-uint64_t odp_time_ns_to_cycles(uint64_t ns);
+uint64_t odp_time_to_u64(odp_time_t time);
--
Regards,
Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
