Change of odp_time_local implementation in odp-dpdk

Message ID CAOdUko13keTzyDvXYb+dBD2sz9LzH6p9HYcWzipYLxg5XpQmsA@mail.gmail.com
State New
Headers show

Commit Message

Wenxian Li April 7, 2016, 11:41 a.m.
Hi Zoltan,

Regarding the odp_time_local api, I made some change leveraging some DPDK
api to avoiding system call.

I have use my change in my project, which benefits the performance.
Below is my diff.

Could you help review it? Any comments would be appreciated.

Thanks,
Wenxian

 static inline int time_cmp(odp_time_t t2, odp_time_t t1)
@@ -103,14 +102,7 @@ static inline void time_wait_until(odp_time_t time)

 static inline uint64_t time_local_res(void)
 {
- int ret;
- struct timespec tres;
-
- ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
- if (odp_unlikely(ret != 0))
- ODP_ABORT("clock_getres failed\n");
-
- return ODP_TIME_SEC_IN_NS / (uint64_t)tres.tv_nsec;
+    return rte_get_tsc_hz();
 }

 odp_time_t odp_time_local(void)
@@ -179,28 +171,14 @@ void odp_time_wait_until(odp_time_t time)

 uint64_t odp_time_to_u64(odp_time_t time)
 {
- int ret;
- struct timespec tres;
- uint64_t resolution;
-
- ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
- if (odp_unlikely(ret != 0))
- ODP_ABORT("clock_getres failed\n");
-
- resolution = (uint64_t)tres.tv_nsec;
-
- return time_to_ns(time) / resolution;
+    return time_to_ns(time) * time_local_res() / ODP_TIME_SEC_IN_NS;
 }

 int odp_time_init_global(void)
 {
- int ret;
- _odp_time_t time;
-
- ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);
- start_time = ret ? ODP_TIME_NULL : time.ex;
-
- return ret;
+    start_time = ODP_TIME_NULL;
+    start_time = odp_time_local();
+    return 0;
 }

 int odp_time_term_global(void)

Comments

Zoltan Kiss April 7, 2016, 2:37 p.m. | #1
Hi Wenxian,

On 07/04/16 12:41, Wenxian Li wrote:
> Hi Zoltan,
>
> Regarding the odp_time_local api, I made some change leveraging some
> DPDK api to avoiding system call.
>
> I have use my change in my project, which benefits the performance.
> Below is my diff.
>
> Could you help review it? Any comments would be appreciated.
>
> Thanks,
> Wenxian
>
> diff --git a/platform/linux-generic/odp_time.c
> b/platform/linux-generic/odp_time.c
> index ca8a955..565ce93 100644
> --- a/platform/linux-generic/odp_time.c
> +++ b/platform/linux-generic/odp_time.c

I'll review the patches next week, but one quick thing: in ODP-DPDK we 
try to avoid any modifications outside platform/linux-dpdk, so we can 
easily pull patches from the odp-linux repo (of course creating new 
files outside this directory doesn't matter, they don't break merge).
To keep that practice, you need to create 2 patches: the first just 
create an identical copy of odp_time.c in linux-dpdk directory, and 
changes the linux-dpdk/Makefile.am to use that. The second applies these 
modifications onto that.
You should:
- save linux-generic/odp_time.c somewhere
- revert it back to original
- create that first commit
- copy your saved file onto linux-dpdk/odp_time.c, and create the second 
commit

> @@ -10,6 +10,7 @@
>   #include <odp/time.h>
>   #include <odp/hints.h>
>   #include <odp_debug_internal.h>
> +#include <rte_cycles.h>
>
>   typedef union {
> odp_time_t      ex;
> @@ -17,6 +18,8 @@ typedef union {
>   } _odp_time_t;
>
>   static odp_time_t start_time;
> +static inline odp_time_t time_local_from_ns(uint64_t ns);
> +static inline uint64_t time_local_res(void);
>
>   static inline
>   uint64_t time_to_ns(odp_time_t time)
> @@ -46,14 +49,10 @@ static inline odp_time_t time_diff(odp_time_t t2,
> odp_time_t t1)
>
>   static inline odp_time_t time_local(void)
>   {
> -int ret;
> -_odp_time_t time;
> -
> -ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in <http://time.in>);
> -if (odp_unlikely(ret != 0))
> -ODP_ABORT("clock_gettime failed\n");
> -
> -return time_diff(time.ex, start_time);
> +    uint64_t ns = ((double)rte_rdtsc()/(double)time_local_res())
> +                  *ODP_TIME_SEC_IN_NS;
> +    odp_time_t time = time_local_from_ns(ns);
> +    return time_diff(time, start_time);
>   }
>
>   static inline int time_cmp(odp_time_t t2, odp_time_t t1)
> @@ -103,14 +102,7 @@ static inline void time_wait_until(odp_time_t time)
>
>   static inline uint64_t time_local_res(void)
>   {
> -int ret;
> -struct timespec tres;
> -
> -ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
> -if (odp_unlikely(ret != 0))
> -ODP_ABORT("clock_getres failed\n");
> -
> -return ODP_TIME_SEC_IN_NS / (uint64_t)tres.tv_nsec;
> +    return rte_get_tsc_hz();
>   }
>
>   odp_time_t odp_time_local(void)
> @@ -179,28 +171,14 @@ void odp_time_wait_until(odp_time_t time)
>
>   uint64_t odp_time_to_u64(odp_time_t time)
>   {
> -int ret;
> -struct timespec tres;
> -uint64_t resolution;
> -
> -ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
> -if (odp_unlikely(ret != 0))
> -ODP_ABORT("clock_getres failed\n");
> -
> -resolution = (uint64_t)tres.tv_nsec;
> -
> -return time_to_ns(time) / resolution;
> +    return time_to_ns(time) * time_local_res() / ODP_TIME_SEC_IN_NS;
>   }
>
>   int odp_time_init_global(void)
>   {
> -int ret;
> -_odp_time_t time;
> -
> -ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in <http://time.in>);
> -start_time = ret ? ODP_TIME_NULL : time.ex;
> -
> -return ret;
> +    start_time = ODP_TIME_NULL;
> +    start_time = odp_time_local();
> +    return 0;
>   }
>
>   int odp_time_term_global(void)

Patch hide | download patch | download mbox

diff --git a/platform/linux-generic/odp_time.c
b/platform/linux-generic/odp_time.c
index ca8a955..565ce93 100644
--- a/platform/linux-generic/odp_time.c
+++ b/platform/linux-generic/odp_time.c
@@ -10,6 +10,7 @@ 
 #include <odp/time.h>
 #include <odp/hints.h>
 #include <odp_debug_internal.h>
+#include <rte_cycles.h>

 typedef union {
  odp_time_t      ex;
@@ -17,6 +18,8 @@  typedef union {
 } _odp_time_t;

 static odp_time_t start_time;
+static inline odp_time_t time_local_from_ns(uint64_t ns);
+static inline uint64_t time_local_res(void);

 static inline
 uint64_t time_to_ns(odp_time_t time)
@@ -46,14 +49,10 @@  static inline odp_time_t time_diff(odp_time_t t2,
odp_time_t t1)

 static inline odp_time_t time_local(void)
 {
- int ret;
- _odp_time_t time;
-
- ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);
- if (odp_unlikely(ret != 0))
- ODP_ABORT("clock_gettime failed\n");
-
- return time_diff(time.ex, start_time);
+    uint64_t ns = ((double)rte_rdtsc()/(double)time_local_res())
+                  *ODP_TIME_SEC_IN_NS;
+    odp_time_t time = time_local_from_ns(ns);
+    return time_diff(time, start_time);
 }