| Message ID | 1438986490-12100-2-git-send-email-ivan.khoronzhuk@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 19.08.15 17:12, Savolainen, Petri (Nokia - FI/Espoo) wrote: > Hi, > > As I mentioned in the call, I'd like to avoid confusion between time and timer (timer tick). In minimum, the API documentation should not refer to "timer". Also could term 'tick' be removed altogether and just use 'time'? > > Something like this: > > uint64_t odp_time(void); > uint64_t odp_time_diff(uint64_t t1, uint64_t t2); > uint64_t odp_time_to_ns(uint64_t time); > uint64_t odp_time_from_ns(uint64_t ns); > > -Petri Sorry I've unintentionally missed this call. It's confusing only at first glance. I'm used to this already. Even don't know, it seems to be reasonable, but any way we have similar function odp_time_to_ns. And avoiding word tick makes the functions a little strange, like you take time, by default it is in ticks, but time is so general word... name odp_time_to_ns sounds like you convert time to ns, how it can be? But if it can make life easier I have no objection, I don't know conclusion about this on the call but if Maxim, Stuart and others are OK with it I can change it. > > >> diff --git a/include/odp/api/time.h b/include/odp/api/time.h >> index b0072fc..b48dcae 100644 >> --- a/include/odp/api/time.h >> +++ b/include/odp/api/time.h >> @@ -30,11 +30,11 @@ extern "C" { >> >> >> /** >> - * Current time in CPU cycles >> + * Current time in ticks of best hi-resolution timer available >> * >> - * @return Current time in CPU cycles >> + * @return Current time in timer ticks >> */ >> -uint64_t odp_time_cycles(void); >> +uint64_t odp_time_tick(void); >> >> >> /** >> @@ -43,29 +43,29 @@ 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 in timer ticks >> */ >> -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2); >> +uint64_t odp_time_ticks_diff(uint64_t t1, uint64_t t2); >> >> >> /** >> - * Convert CPU cycles to nanoseconds >> + * Convert timer ticks to nanoseconds >> * >> - * @param cycles Time in CPU cycles >> + * @param ticks Time in timer ticks >> * >> * @return Time in nanoseconds >> */ >> -uint64_t odp_time_cycles_to_ns(uint64_t cycles); >> +uint64_t odp_time_tick_to_ns(uint64_t ticks); >> >> >> /** >> - * Convert nanoseconds to CPU cycles >> + * Convert nanoseconds to ticks of best hi-resolution timer available >> * >> * @param ns Time in nanoseconds >> * >> - * @return Time in CPU cycles >> + * @return Time in timer ticks >> */ >> -uint64_t odp_time_ns_to_cycles(uint64_t ns); >> +uint64_t odp_time_ns_to_tick(uint64_t ns);
+ On 19.08.15 17:49, Ivan Khoronzhuk wrote: > > > On 19.08.15 17:12, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> Hi, >> >> As I mentioned in the call, I'd like to avoid confusion between time and timer (timer tick). In minimum, the API documentation should not refer to "timer". Also could term 'tick' be removed altogether and just use 'time'? >> >> Something like this: >> >> uint64_t odp_time(void); >> uint64_t odp_time_diff(uint64_t t1, uint64_t t2); >> uint64_t odp_time_to_ns(uint64_t time); >> uint64_t odp_time_from_ns(uint64_t ns); >> >> -Petri > > Sorry I've unintentionally missed this call. > > It's confusing only at first glance. I'm used to this already. > Even don't know, it seems to be reasonable, but any way we > have similar function odp_time_to_ns. And avoiding word tick > makes the functions a little strange, like you take time, by default > it is in ticks, but time is so general word... name odp_time_to_ns > sounds like you convert time to ns, how it can be? > > But if it can make life easier I have no objection, I don't know conclusion > about this on the call but if Maxim, Stuart and others are OK with it I > can change it. > Maybe odp_tick() odp_tick_diff() odp_tick_to_ns() odp_tick_from_ns() .... >> >> >>> diff --git a/include/odp/api/time.h b/include/odp/api/time.h >>> index b0072fc..b48dcae 100644 >>> --- a/include/odp/api/time.h >>> +++ b/include/odp/api/time.h >>> @@ -30,11 +30,11 @@ extern "C" { >>> >>> >>> /** >>> - * Current time in CPU cycles >>> + * Current time in ticks of best hi-resolution timer available >>> * >>> - * @return Current time in CPU cycles >>> + * @return Current time in timer ticks >>> */ >>> -uint64_t odp_time_cycles(void); >>> +uint64_t odp_time_tick(void); >>> >>> >>> /** >>> @@ -43,29 +43,29 @@ 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 in timer ticks >>> */ >>> -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2); >>> +uint64_t odp_time_ticks_diff(uint64_t t1, uint64_t t2); >>> >>> >>> /** >>> - * Convert CPU cycles to nanoseconds >>> + * Convert timer ticks to nanoseconds >>> * >>> - * @param cycles Time in CPU cycles >>> + * @param ticks Time in timer ticks >>> * >>> * @return Time in nanoseconds >>> */ >>> -uint64_t odp_time_cycles_to_ns(uint64_t cycles); >>> +uint64_t odp_time_tick_to_ns(uint64_t ticks); >>> >>> >>> /** >>> - * Convert nanoseconds to CPU cycles >>> + * Convert nanoseconds to ticks of best hi-resolution timer available >>> * >>> * @param ns Time in nanoseconds >>> * >>> - * @return Time in CPU cycles >>> + * @return Time in timer ticks >>> */ >>> -uint64_t odp_time_ns_to_cycles(uint64_t ns); >>> +uint64_t odp_time_ns_to_tick(uint64_t ns); >
On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote: > Since it's time API, the prefix should be "odp_time_" > > Maybe we need to use an abstract time type (odp_time_t) to make it clear that application should not directly calculate time (or ticks) but use APIs. We could add other (to_sec, to_ms, ...) conversion calls later. > > odp_time_t odp_time(void); > odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); > uint64_t odp_time_to_ns(odp_time_t time); > odp_time_t odp_time_from_ns(uint64_t ns); > > Additionally, resolution maybe interesting (since it may vary a lot: from 1ns to tens of ms) > > // Time resolution in nanoseconds > uint64_t odp_time_resolution_ns(void); > > > -Petri > > I'm Ok to change it ot odp_time. Stuart, Maxim what do you say? According resolution, I had sent patch based on this series "validation: time: use timer resolution instead of TOLERANCE" https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html As an example it uses odp_time_tick_to_ns(1) to get resolution. I think no need in this API. > >> -----Original Message----- >> From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >> Sent: Wednesday, August 19, 2015 5:53 PM >> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >> Stuart Haslam; Maxim Uvarov >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU >> cycles from time API >> >> + >> >> On 19.08.15 17:49, Ivan Khoronzhuk wrote: >>> >>> >>> On 19.08.15 17:12, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>> Hi, >>>> >>>> As I mentioned in the call, I'd like to avoid confusion between time >> and timer (timer tick). In minimum, the API documentation should not >> refer to "timer". Also could term 'tick' be removed altogether and just >> use 'time'? >>>> >>>> Something like this: >>>> >>>> uint64_t odp_time(void); >>>> uint64_t odp_time_diff(uint64_t t1, uint64_t t2); >>>> uint64_t odp_time_to_ns(uint64_t time); >>>> uint64_t odp_time_from_ns(uint64_t ns); >>>> >>>> -Petri >>> >>> Sorry I've unintentionally missed this call. >>> >>> It's confusing only at first glance. I'm used to this already. >>> Even don't know, it seems to be reasonable, but any way we >>> have similar function odp_time_to_ns. And avoiding word tick >>> makes the functions a little strange, like you take time, by default >>> it is in ticks, but time is so general word... name odp_time_to_ns >>> sounds like you convert time to ns, how it can be? >>> >>> But if it can make life easier I have no objection, I don't know >> conclusion >>> about this on the call but if Maxim, Stuart and others are OK with it >> I >>> can change it. >>> >> >> Maybe >> odp_tick() >> odp_tick_diff() >> odp_tick_to_ns() >> odp_tick_from_ns() >> .... >> >> >>>> >>>> >>>>> diff --git a/include/odp/api/time.h b/include/odp/api/time.h >>>>> index b0072fc..b48dcae 100644 >>>>> --- a/include/odp/api/time.h >>>>> +++ b/include/odp/api/time.h >>>>> @@ -30,11 +30,11 @@ extern "C" { >>>>> >>>>> >>>>> /** >>>>> - * Current time in CPU cycles >>>>> + * Current time in ticks of best hi-resolution timer available >>>>> * >>>>> - * @return Current time in CPU cycles >>>>> + * @return Current time in timer ticks >>>>> */ >>>>> -uint64_t odp_time_cycles(void); >>>>> +uint64_t odp_time_tick(void); >>>>> >>>>> >>>>> /** >>>>> @@ -43,29 +43,29 @@ 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 in timer ticks >>>>> */ >>>>> -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2); >>>>> +uint64_t odp_time_ticks_diff(uint64_t t1, uint64_t t2); >>>>> >>>>> >>>>> /** >>>>> - * Convert CPU cycles to nanoseconds >>>>> + * Convert timer ticks to nanoseconds >>>>> * >>>>> - * @param cycles Time in CPU cycles >>>>> + * @param ticks Time in timer ticks >>>>> * >>>>> * @return Time in nanoseconds >>>>> */ >>>>> -uint64_t odp_time_cycles_to_ns(uint64_t cycles); >>>>> +uint64_t odp_time_tick_to_ns(uint64_t ticks); >>>>> >>>>> >>>>> /** >>>>> - * Convert nanoseconds to CPU cycles >>>>> + * Convert nanoseconds to ticks of best hi-resolution timer >> available >>>>> * >>>>> * @param ns Time in nanoseconds >>>>> * >>>>> - * @return Time in CPU cycles >>>>> + * @return Time in timer ticks >>>>> */ >>>>> -uint64_t odp_time_ns_to_cycles(uint64_t ns); >>>>> +uint64_t odp_time_ns_to_tick(uint64_t ns); >>> >> >> -- >> Regards, >> Ivan Khoronzhuk
On 20.08.15 17:17, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -----Original Message----- >> From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >> Sent: Thursday, August 20, 2015 4:49 PM >> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >> Stuart Haslam; Maxim Uvarov >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU >> cycles from time API >> >> >> On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>> Since it's time API, the prefix should be "odp_time_" >>> >>> Maybe we need to use an abstract time type (odp_time_t) to make it >> clear that application should not directly calculate time (or ticks) >> but use APIs. We could add other (to_sec, to_ms, ...) conversion calls >> later. >>> >>> odp_time_t odp_time(void); >>> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); >>> uint64_t odp_time_to_ns(odp_time_t time); >>> odp_time_t odp_time_from_ns(uint64_t ns); >>> >>> Additionally, resolution maybe interesting (since it may vary a lot: >> from 1ns to tens of ms) >>> >>> // Time resolution in nanoseconds >>> uint64_t odp_time_resolution_ns(void); >>> >>> >>> -Petri >>> >>> >> >> I'm Ok to change it ot odp_time. >> Stuart, Maxim what do you say? >> >> According resolution, I had sent patch based on this series >> "validation: time: use timer resolution instead of TOLERANCE" >> https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html >> As an example it uses odp_time_tick_to_ns(1) to get resolution. >> I think no need in this API. > > With opaque types (odp_time_t), > > odp_time_to_ns(1); > > > would be an illegal call. Time value would be packed into odp_time_t in an implementation specific way. > Application would calculate in nanosec (uint64_t), and would use the API to convert between nsec and odp_time_t. Can be..not sure about odp_time_t, seems it's logical, it asks us to use API and abstracts time from units. But it requires a little more changes...and it be good to know if others are OK before changing it. > > -Petri > > > >
On Thu, Aug 20, 2015 at 05:58:09PM +0300, Ivan Khoronzhuk wrote: > > > On 20.08.15 17:17, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > > >>-----Original Message----- > >>From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] > >>Sent: Thursday, August 20, 2015 4:49 PM > >>To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; > >>Stuart Haslam; Maxim Uvarov > >>Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU > >>cycles from time API > >> > >> > >>On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote: > >>>Since it's time API, the prefix should be "odp_time_" > >>> > >>>Maybe we need to use an abstract time type (odp_time_t) to make it > >>clear that application should not directly calculate time (or ticks) > >>but use APIs. We could add other (to_sec, to_ms, ...) conversion calls > >>later. > >>> > >>>odp_time_t odp_time(void); > >>>odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); > >>>uint64_t odp_time_to_ns(odp_time_t time); > >>>odp_time_t odp_time_from_ns(uint64_t ns); > >>> > >>>Additionally, resolution maybe interesting (since it may vary a lot: > >>from 1ns to tens of ms) Really? I had been thinking of a resolution somewhere between roughly < 1ns (multiple GHz as in a CPU cycle counter) to around 100ns (10MHz). Having a resolution of tens of ms would be limiting for applications that use this for interval timing, e.g. odp_pktio_perf uses it for inter-burst gap timing. > >>> > >>>// Time resolution in nanoseconds > >>>uint64_t odp_time_resolution_ns(void); > >>> > >>> > >>>-Petri > >>> > >>> > >> > >>I'm Ok to change it ot odp_time. > >>Stuart, Maxim what do you say? > >> > >>According resolution, I had sent patch based on this series > >>"validation: time: use timer resolution instead of TOLERANCE" > >>https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html > >>As an example it uses odp_time_tick_to_ns(1) to get resolution. > >>I think no need in this API. > > > >With opaque types (odp_time_t), > > > >odp_time_to_ns(1); > > > > > >would be an illegal call. Time value would be packed into odp_time_t in an implementation specific way. > >Application would calculate in nanosec (uint64_t), and would use the API to convert between nsec and odp_time_t. > > Can be..not sure about odp_time_t, seems it's logical, it asks us to use API and abstracts time from units. > But it requires a little more changes...and it be good to know if others are OK before changing it. > The suggested changes seem sensible to me, the application should be working with wall-clock units so removing the base units makes that a bit clearer.
On 21 August 2015 at 03:53, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> wrote: > > >> -----Original Message----- >> From: ext Stuart Haslam [mailto:stuart.haslam@linaro.org] >> Sent: Thursday, August 20, 2015 7:41 PM >> To: Ivan Khoronzhuk >> Cc: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >> Maxim Uvarov >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU >> cycles from time API >> >> On Thu, Aug 20, 2015 at 05:58:09PM +0300, Ivan Khoronzhuk wrote: >> > >> > >> > On 20.08.15 17:17, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> > > >> > > >> > >>-----Original Message----- >> > >>From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >> > >>Sent: Thursday, August 20, 2015 4:49 PM >> > >>To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >> > >>Stuart Haslam; Maxim Uvarov >> > >>Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind >> CPU > > > >> > >>>Additionally, resolution maybe interesting (since it may vary a >> lot: >> > >>from 1ns to tens of ms) >> >> Really? I had been thinking of a resolution somewhere between roughly < >> 1ns >> (multiple GHz as in a CPU cycle counter) to around 100ns (10MHz). >> >> Having a resolution of tens of ms would be limiting for applications >> that use this for interval timing, e.g. odp_pktio_perf uses it for >> inter-burst gap timing. > > Depends on the time source. It would be bad implementation, but someone could just implement it with > gettimeofday() (or some other non-high frequency clock from the OS). > > As you noted, it would be good it application could notice the (bad) resolution and decide what to do with it (complain, try to use CPU cycle counter instead, ...) Can determining the resolution come from the unit tests when an implementation is checked for it conformance to the API, or does there need to be something available to applications > > -Petri > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
Petri, On 20.08.15 17:58, Ivan Khoronzhuk wrote: > > > On 20.08.15 17:17, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> >> >>> -----Original Message----- >>> From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >>> Sent: Thursday, August 20, 2015 4:49 PM >>> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >>> Stuart Haslam; Maxim Uvarov >>> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU >>> cycles from time API >>> >>> >>> On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>> Since it's time API, the prefix should be "odp_time_" >>>> >>>> Maybe we need to use an abstract time type (odp_time_t) to make it >>> clear that application should not directly calculate time (or ticks) >>> but use APIs. We could add other (to_sec, to_ms, ...) conversion calls >>> later. >>>> >>>> odp_time_t odp_time(void); >>>> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); >>>> uint64_t odp_time_to_ns(odp_time_t time); >>>> odp_time_t odp_time_from_ns(uint64_t ns); >>>> >>>> Additionally, resolution maybe interesting (since it may vary a lot: >>> from 1ns to tens of ms) >>>> >>>> // Time resolution in nanoseconds >>>> uint64_t odp_time_resolution_ns(void); >>>> >>>> >>>> -Petri >>>> >>>> >>> >>> I'm Ok to change it ot odp_time. >>> Stuart, Maxim what do you say? >>> >>> According resolution, I had sent patch based on this series >>> "validation: time: use timer resolution instead of TOLERANCE" >>> https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html >>> As an example it uses odp_time_tick_to_ns(1) to get resolution. >>> I think no need in this API. >> >> With opaque types (odp_time_t), >> >> odp_time_to_ns(1); >> >> >> would be an illegal call. Time value would be packed into odp_time_t in an implementation specific way. >> Application would calculate in nanosec (uint64_t), and would use the API to convert between nsec and odp_time_t. > > Can be..not sure about odp_time_t, Adding opaque type requires to add function like odp_time_cmp(odp_time_t t1, odp_time_t t2) to be able to compare times. Also it may require to change scheduler API as it uses time in it. odp_event_t odp_schedule(odp_queue_t *from, uint64_t wait); Also it supposes to support ODP_SCHED_WAIT and ODP_SCHED_NO_WAIT which is not part of odp_time_t.... How are we going to convert schedule time to odp_time_t? The time and schedule time were mixed everywhere in the code. It's not correct to have two types of time...and probably change like odp_event_t odp_schedule(odp_queue_t *from, uint64_t wait) to odp_event_t odp_schedule(odp_queue_t *from, odp_time_t time) requires us to add call like odp_schedule_wait(odp_queue_t *from) For instance: example/ipsec/odp_ipsec.c polled_odp_schedule(odp_queue_t *from, odp_time_t wait) Do we really need this, :-|? seems it's logical, it asks us to use API and abstracts time from units. > But it requires a little more changes...and it be good to know if others are OK before changing it. > >> >> -Petri >> >> >> >> >
Petri, On 26.08.15 14:46, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -----Original Message----- >> From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >> Sent: Wednesday, August 26, 2015 1:22 PM >> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >> Stuart Haslam; Maxim Uvarov >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU >> cycles from time API >> >> Petri, >> >> On 20.08.15 17:58, Ivan Khoronzhuk wrote: >>> >>> >>> On 20.08.15 17:17, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>> >>>> >>>>> -----Original Message----- >>>>> From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >>>>> Sent: Thursday, August 20, 2015 4:49 PM >>>>> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >>>>> Stuart Haslam; Maxim Uvarov >>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind >> CPU >>>>> cycles from time API >>>>> >>>>> >>>>> On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>>>> Since it's time API, the prefix should be "odp_time_" >>>>>> >>>>>> Maybe we need to use an abstract time type (odp_time_t) to make it >>>>> clear that application should not directly calculate time (or >> ticks) >>>>> but use APIs. We could add other (to_sec, to_ms, ...) conversion >> calls >>>>> later. >>>>>> >>>>>> odp_time_t odp_time(void); >>>>>> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); >>>>>> uint64_t odp_time_to_ns(odp_time_t time); >>>>>> odp_time_t odp_time_from_ns(uint64_t ns); >>>>>> >>>>>> Additionally, resolution maybe interesting (since it may vary a >> lot: >>>>> from 1ns to tens of ms) >>>>>> >>>>>> // Time resolution in nanoseconds >>>>>> uint64_t odp_time_resolution_ns(void); >>>>>> >>>>>> >>>>>> -Petri >>>>>> >>>>>> >>>>> >>>>> I'm Ok to change it ot odp_time. >>>>> Stuart, Maxim what do you say? >>>>> >>>>> According resolution, I had sent patch based on this series >>>>> "validation: time: use timer resolution instead of TOLERANCE" >>>>> https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html >>>>> As an example it uses odp_time_tick_to_ns(1) to get resolution. >>>>> I think no need in this API. >>>> >>>> With opaque types (odp_time_t), >>>> >>>> odp_time_to_ns(1); >>>> >>>> >>>> would be an illegal call. Time value would be packed into odp_time_t >> in an implementation specific way. >>>> Application would calculate in nanosec (uint64_t), and would use the >> API to convert between nsec and odp_time_t. >>> >>> Can be..not sure about odp_time_t, >> >> >> Adding opaque type requires to add function like >> odp_time_cmp(odp_time_t t1, odp_time_t t2) >> to be able to compare times. > > It's the diff How? Example, please. diff is diff. It returns time diff always. It cannot compare t1 and t2, it takes in account overflows. cmp is cmp. It acts simpler, it doesn't take into account overflows. It simply compare t1 and t2 (time ranges for instance), smth like: return -1 if t1 > t2 return 0 if t1 = t2 return 1 if ti < t2. > > odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); > >> Also it may require to change scheduler API as it uses time in it. >> >> odp_event_t odp_schedule(odp_queue_t *from, uint64_t wait); >> >> Also it supposes to support ODP_SCHED_WAIT and ODP_SCHED_NO_WAIT >> which is not part of odp_time_t.... >> >> How are we going to convert schedule time to odp_time_t? >> The time and schedule time were mixed everywhere in the code. > > Scheduler (wait time), time (wall clock) and timer are separate APIs Thanks for reminding, but that's why my reply. > and time/ticks defined in those should not be mixed. They have been mixing. That's the issue I'm talking about. I intentionally have pointed on example. Once again, for instance: example/ipsec/odp_ipsec.c polled_odp_schedule(odp_queue_t *from, uint64_t wait) It's a good question if scheduler wait time should use odp_time_t type also. That is the main question, in another case nsec->sched_time->nsec->wall_time conversions must be used. Strange conversion but it's required. See above example, it's good example where you may use compare function and apply strange conversion in question. > Currently, both have their own nsec->time conversion functions, so that the implementation underneath can be optimized to use scheduler ticks vs. wall clock ticks, which may have different resolution and other requirements, etc. > There's no immediate need to change (no bug in) scheduler API. I can avoid scheduler API change. It's not a problem, and I didn't propose to change it immediately. My intention here is to point that it can be changed to match those bad examples that were written before I saw them, maybe... seems like, it was written bearing in mind that sched time and wall time is the same time. (Please pay attention, it's example, not implementation side.). So, maybe it was not clear enough, when it was written. Now, seems that is time to clarify that and fix. > > -Petri > > >> It's not correct to have two types of time...and probably change like >> >> odp_event_t odp_schedule(odp_queue_t *from, uint64_t wait) >> to >> odp_event_t odp_schedule(odp_queue_t *from, odp_time_t time) >> >> requires us to add call like odp_schedule_wait(odp_queue_t *from) >> >> For instance: example/ipsec/odp_ipsec.c >> polled_odp_schedule(odp_queue_t *from, odp_time_t wait) >> >> Do we really need this, :-|? >> >> >> seems it's logical, it asks us to use API and abstracts time from >> units. >>> But it requires a little more changes...and it be good to know if >> others are OK before changing it. >>> >>>> >>>> -Petri >>>> >>>> >>>> >>>> >>> >> >> -- >> Regards, >> Ivan Khoronzhuk
On 26.08.15 15:40, Ivan Khoronzhuk wrote: > Petri, > > On 26.08.15 14:46, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> >> >>> -----Original Message----- >>> From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >>> Sent: Wednesday, August 26, 2015 1:22 PM >>> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >>> Stuart Haslam; Maxim Uvarov >>> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU >>> cycles from time API >>> >>> Petri, >>> >>> On 20.08.15 17:58, Ivan Khoronzhuk wrote: >>>> >>>> >>>> On 20.08.15 17:17, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >>>>>> Sent: Thursday, August 20, 2015 4:49 PM >>>>>> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >>>>>> Stuart Haslam; Maxim Uvarov >>>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind >>> CPU >>>>>> cycles from time API >>>>>> >>>>>> >>>>>> On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>>>>> Since it's time API, the prefix should be "odp_time_" >>>>>>> >>>>>>> Maybe we need to use an abstract time type (odp_time_t) to make it >>>>>> clear that application should not directly calculate time (or >>> ticks) >>>>>> but use APIs. We could add other (to_sec, to_ms, ...) conversion >>> calls >>>>>> later. >>>>>>> >>>>>>> odp_time_t odp_time(void); >>>>>>> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); >>>>>>> uint64_t odp_time_to_ns(odp_time_t time); >>>>>>> odp_time_t odp_time_from_ns(uint64_t ns); >>>>>>> >>>>>>> Additionally, resolution maybe interesting (since it may vary a >>> lot: >>>>>> from 1ns to tens of ms) >>>>>>> >>>>>>> // Time resolution in nanoseconds >>>>>>> uint64_t odp_time_resolution_ns(void); >>>>>>> >>>>>>> >>>>>>> -Petri >>>>>>> >>>>>>> >>>>>> >>>>>> I'm Ok to change it ot odp_time. >>>>>> Stuart, Maxim what do you say? >>>>>> >>>>>> According resolution, I had sent patch based on this series >>>>>> "validation: time: use timer resolution instead of TOLERANCE" >>>>>> https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html >>>>>> As an example it uses odp_time_tick_to_ns(1) to get resolution. >>>>>> I think no need in this API. >>>>> >>>>> With opaque types (odp_time_t), >>>>> >>>>> odp_time_to_ns(1); >>>>> >>>>> >>>>> would be an illegal call. Time value would be packed into odp_time_t >>> in an implementation specific way. >>>>> Application would calculate in nanosec (uint64_t), and would use the >>> API to convert between nsec and odp_time_t. >>>> >>>> Can be..not sure about odp_time_t, >>> >>> >>> Adding opaque type requires to add function like >>> odp_time_cmp(odp_time_t t1, odp_time_t t2) >>> to be able to compare times. >> >> It's the diff > > How? Example, please. > > diff is diff. It returns time diff always. > It cannot compare t1 and t2, it takes in account overflows. > > cmp is cmp. It acts simpler, it doesn't take into account overflows. > It simply compare t1 and t2 (time ranges for instance), smth like: > return -1 if t1 > t2 > return 0 if t1 = t2 > return 1 if ti < t2. Petry, are you OK, with this? odp_time_cmp() is not only function that is required to add in flavor of opaque type. In general we need several functions that allow to work with odp_time_t: /** * Compare two times as absolute ranges * * @param t1 First time * @param t2 Second time * * @retval -1 if t1 < t2, 0 if t1 = t2, 1 if t1 > t2 */ int odp_time_cmp(odp_time_t t1, odp_time_t t2); /** * Time sum * * @param t1 First time stamp * @param t2 Second time stamp * * @return Sum of time stamps in time handles */ odp_time_t odp_time_add(odp_time_t t1, odp_time_t t2); /** * Get printable value for an odp_time_t * * @param hdl odp_time_t handle to be printed * @return uint64_t value that can be used to print/display this * handle * * @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 handle. */ uint64_t odp_time_to_u64(odp_time_t hdl); Maybe we shouldn't bother with it and use uint64_t? In this case we don't need additional API and can save cycles eliminating redundant API? > >> >> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); >> >>> Also it may require to change scheduler API as it uses time in it. >>> >>> odp_event_t odp_schedule(odp_queue_t *from, uint64_t wait); >>> >>> Also it supposes to support ODP_SCHED_WAIT and ODP_SCHED_NO_WAIT >>> which is not part of odp_time_t.... >>> >>> How are we going to convert schedule time to odp_time_t? >>> The time and schedule time were mixed everywhere in the code. >> >> Scheduler (wait time), time (wall clock) and timer are separate APIs > > Thanks for reminding, but that's why my reply. > >> and time/ticks defined in those should not be mixed. > > They have been mixing. That's the issue I'm talking about. > I intentionally have pointed on example. > Once again, for instance: example/ipsec/odp_ipsec.c > polled_odp_schedule(odp_queue_t *from, uint64_t wait) > > It's a good question if scheduler wait time should use odp_time_t type also. > > That is the main question, in another case nsec->sched_time->nsec->wall_time conversions must be used. > Strange conversion but it's required. See above example, it's good example > where you may use compare function and apply strange conversion in question. > >> Currently, both have their own nsec->time conversion functions, so that the implementation underneath can be optimized to use scheduler ticks vs. wall clock ticks, which may have different resolution and other requirements, etc. >> There's no immediate need to change (no bug in) scheduler API. > > I can avoid scheduler API change. It's not a problem, and I didn't propose to change it immediately. > My intention here is to point that it can be changed to match those bad examples that were > written before I saw them, maybe... seems like, it was written bearing in mind that sched time and wall time is > the same time. (Please pay attention, it's example, not implementation side.). So, maybe > it was not clear enough, when it was written. Now, seems that is time to clarify that and fix. > >> >> -Petri >> >> >>> It's not correct to have two types of time...and probably change like >>> >>> odp_event_t odp_schedule(odp_queue_t *from, uint64_t wait) >>> to >>> odp_event_t odp_schedule(odp_queue_t *from, odp_time_t time) >>> >>> requires us to add call like odp_schedule_wait(odp_queue_t *from) >>> >>> For instance: example/ipsec/odp_ipsec.c >>> polled_odp_schedule(odp_queue_t *from, odp_time_t wait) >>> >>> Do we really need this, :-|? >>> >>> >>> seems it's logical, it asks us to use API and abstracts time from >>> units. >>>> But it requires a little more changes...and it be good to know if >>> others are OK before changing it. >>>> >>>>> >>>>> -Petri >>>>> >>>>> >>>>> >>>>> >>>> >>> >>> -- >>> Regards, >>> Ivan Khoronzhuk >
Petri, On 27.08.15 17:10, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>>> >>>>> Adding opaque type requires to add function like >>>>> odp_time_cmp(odp_time_t t1, odp_time_t t2) >>>>> to be able to compare times. >>>> >>>> It's the diff >>> >>> How? Example, please. >>> >>> diff is diff. It returns time diff always. >>> It cannot compare t1 and t2, it takes in account overflows. >>> >>> cmp is cmp. It acts simpler, it doesn't take into account overflows. >>> It simply compare t1 and t2 (time ranges for instance), smth like: >>> return -1 if t1 > t2 >>> return 0 if t1 = t2 >>> return 1 if ti < t2. > > > OK, not only calculate diff but e.g. compare two diffs. > >> >> Petry, are you OK, with this? >> >> odp_time_cmp() is not only function that is required to add in flavor >> of opaque type. >> In general we need several functions that allow to work with >> odp_time_t: >> >> /** >> * Compare two times as absolute ranges >> * >> * @param t1 First time >> * @param t2 Second time >> * >> * @retval -1 if t1 < t2, 0 if t1 = t2, 1 if t1 > t2 >> */ >> int odp_time_cmp(odp_time_t t1, odp_time_t t2); >> >> /** >> * Time sum >> * >> * @param t1 First time stamp >> * @param t2 Second time stamp >> * >> * @return Sum of time stamps in time handles >> */ >> odp_time_t odp_time_add(odp_time_t t1, odp_time_t t2); >> >> /** >> * Get printable value for an odp_time_t >> * >> * @param hdl odp_time_t handle to be printed >> * @return uint64_t value that can be used to print/display this >> * handle >> * >> * @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 handle. >> */ >> uint64_t odp_time_to_u64(odp_time_t hdl); >> >> Maybe we shouldn't bother with it and use uint64_t? >> In this case we don't need additional API and can save cycles >> eliminating redundant API? > > > Yes, API gets heavier. Ok, I have been preparing patches. > But on the other hand, getting the time stamp must be fast, so the return value unit needs to be close to native clock source units. Conversion to nsec may take extra calculation, which needs to be avoided on the timestamp call. > > Application could convert interesting time values into odp_time_t beforehand (e.g. 100ms to odp_time) and then request timestamps and add/sub/compare/diff those most of the time. Conversion to/from nsec (or other time units) could be done rarely. Overhead from abstract type should not be significant, but would give freedom for implementation and time format/source used. Actually I meant odp_time_cmp() and odp_time_add() calls, they can be used frequently, in the cycles. conversions ns-ticks-ns, it's OK. > > We'd need also to add requirement that this is global time (timestamps from different threads can be compared). No. We cannot guarantee it, and I no see reason for that. Each CPU can have it`s own h/w counter. If we can guarantee that a thread runs on the same CPU always, no problem. As I understand we can. > > -Petri > >
Hi Petry, On 28.08.15 12:53, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -----Original Message----- >> From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >> Sent: Thursday, August 27, 2015 5:40 PM >> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >> Stuart Haslam; Maxim Uvarov >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU >> cycles from time API >> >> Petri, >> >> On 27.08.15 17:10, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>>>>> >>>>>>> Adding opaque type requires to add function like >>>>>>> odp_time_cmp(odp_time_t t1, odp_time_t t2) >>>>>>> to be able to compare times. >>>>>> >>>>>> It's the diff >>>>> >>>>> How? Example, please. >>>>> >>>>> diff is diff. It returns time diff always. >>>>> It cannot compare t1 and t2, it takes in account overflows. >>>>> >>>>> cmp is cmp. It acts simpler, it doesn't take into account >> overflows. >>>>> It simply compare t1 and t2 (time ranges for instance), smth like: >>>>> return -1 if t1 > t2 >>>>> return 0 if t1 = t2 >>>>> return 1 if ti < t2. >>> >>> >>> OK, not only calculate diff but e.g. compare two diffs. >>> >>>> >>>> Petry, are you OK, with this? >>>> >>>> odp_time_cmp() is not only function that is required to add in >> flavor >>>> of opaque type. >>>> In general we need several functions that allow to work with >>>> odp_time_t: >>>> >>>> /** >>>> * Compare two times as absolute ranges >>>> * >>>> * @param t1 First time >>>> * @param t2 Second time >>>> * >>>> * @retval -1 if t1 < t2, 0 if t1 = t2, 1 if t1 > t2 >>>> */ >>>> int odp_time_cmp(odp_time_t t1, odp_time_t t2); >>>> >>>> /** >>>> * Time sum >>>> * >>>> * @param t1 First time stamp >>>> * @param t2 Second time stamp >>>> * >>>> * @return Sum of time stamps in time handles >>>> */ >>>> odp_time_t odp_time_add(odp_time_t t1, odp_time_t t2); >>>> >>>> /** >>>> * Get printable value for an odp_time_t >>>> * >>>> * @param hdl odp_time_t handle to be printed >>>> * @return uint64_t value that can be used to print/display >> this >>>> * handle >>>> * >>>> * @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 handle. >>>> */ >>>> uint64_t odp_time_to_u64(odp_time_t hdl); >>>> >>>> Maybe we shouldn't bother with it and use uint64_t? >>>> In this case we don't need additional API and can save cycles >>>> eliminating redundant API? >>> >>> >>> Yes, API gets heavier. >> >> Ok, I have been preparing patches. >> >>> But on the other hand, getting the time stamp must be fast, so the >> return value unit needs to be close to native clock source units. >> Conversion to nsec may take extra calculation, which needs to be >> avoided on the timestamp call. >>> >>> Application could convert interesting time values into odp_time_t >> beforehand (e.g. 100ms to odp_time) and then request timestamps and >> add/sub/compare/diff those most of the time. Conversion to/from nsec >> (or other time units) could be done rarely. Overhead from abstract type >> should not be significant, but would give freedom for implementation >> and time format/source used. >> >> Actually I meant odp_time_cmp() and odp_time_add() calls, >> they can be used frequently, in the cycles. >> >> conversions ns-ticks-ns, it's OK. >> >>> >>> We'd need also to add requirement that this is global time >> (timestamps from different threads can be compared). >> >> No. We cannot guarantee it, and I no see reason for that. >> >> Each CPU can have it`s own h/w counter. If we can guarantee that >> a thread runs on the same CPU always, no problem. As I understand we >> can. > > > By default, it should be global time. The reason is that: timestamps from different threads can be compared. > > You can use CPU local counter for global time, if the counter can be synchronized (e.g. copy global time into local counter in odp_init_local) and it keeps in synch with other CPUs (threads). We should actually support two time values: global and thread local time. There are use cases for both. Global: timestamp log entries, etc things which need global (ODP instance level) time ordering. Local: timestamp and measure processing stages within a single thread. > > In summary the new time.h API could look like this: > --------------------------------------------------- > > // abstract time type > // includes information of time value type (global vs. local) > typedef odp_time_t > > // get global time > // global time is common over all threads. Can compare timestamps between threads. > odp_time_t odp_time(void); > > // get thread local time > // thread local time is only meaningful per thread. Cannot compare timestamps from other threads. > // May run from different clock source and different rate than global time. > odp_time_t odp_time_local(void); > > // Compare time values > // -1: t2 < t1 > // 0: t2 == t1 > // 1: t2 > t1 > int odp_time_cmp(odp_time_t t1, odp_time_t t2); > > // Sum of t1 and t2 > odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2); > > // Difference t1 from t2 > // t2 >= t1. Use cmp() first, if don't know which one is the later (t2) > odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); > > // convert time to nsec > // nsec value should start from 0 in ODP init (nsec value must not wrap in several years) > // Global and local time may result different nsec values (different time) > uint64_t odp_time_to_ns(odp_time_t time); > > // convert nsec value to global time > odp_time_t odp_time_from_ns(uint64_t ns); > > // convert nsec value to local time > odp_time_t odp_time_local_from_ns(uint64_t ns); > > // Time info structure > typedef struct { > uint64_t global_hz; // Global timestamp resolution in hz > uint64_t local_hz; // Local timestamp resolution in hz > } odp_time_info_t; > > // Time info request > // Fill in global and local timestamp resolutions in hz > // Can be extended with other time/timestamp info later on > // 0 on success > // <0 on failure > int odp_time_info(odp_time_info_t *info); > > > -Petri > > I didn't see any word about global time in API, but also I didn't see any word that it has to be local. So I supposed that application should think that it's worst case - local. So, if it has to be global, it definitely should be described in the API. I only worry about that the striker requirement the less portability. I've created the above API (including "sum" word as apposite to "diff"). Except global/local differentiation and resolution. Also, I've added new definition that represents 0 ticks for time - ODP_TIME_NULL. It can be used instead of odp_time_from_ns(0), and is useful for comparison and initialization. Also odp_time_to_u64() to get "real" ticks where it's needed. /** * @def ODP_TIME_NULL * Zero time handle */ /** * Current time handle of best hi-resolution time source available * * @return Time handle on success * @retval ODP_TIME_INVALID on failure and errno set. */ odp_time_t odp_time(void); /** * Time difference * * @param t1 First time stamp * @param t2 Second time stamp * * @return Difference of time stamps in time handles */ odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); /** * Time sum * * @param t1 First time stamp * @param t2 Second time stamp * * @return Sum of time stamps in time handles */ odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2); /** * Convert time handle to nanoseconds * * @param time Time handle representing time * * @return Time in nanoseconds */ uint64_t odp_time_to_ns(odp_time_t time); /** * Convert nanoseconds to time of best hi-resolution time source available * * @param ns Time in nanoseconds * * @return Time handle representing time */ 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 t1 < t2, 0 if t1 = t2, 1 if t1 > t2 */ int odp_time_cmp(odp_time_t t1, odp_time_t t2); /** * Get printable value for an odp_time_t * * @param hdl odp_time_t handle to be printed * @return uint64_t value that can be used to print/display this * handle * * @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 handle. */ uint64_t odp_time_to_u64(odp_time_t hdl); According resolution. I'm changing existent API, not create new one, so firstly I want to add changes required after API change and then add resolution series that will add resolution API and correct places where it should be used. According global/local time. As I see, there can be corner cases: - no global timer, but it can be emulated by local (start local timers synchronously or in another way) - global timer but no local. In this case local timer can be emulated by global. We can replace local on global, global on local. If so we can always use global time source and don't bother with local/global, And if they have different resolution - choose the best and emulate if needed. I mean that main criteria is resolution. If platform cannot emulate it for some reason, then we can have platforms that cannot implement global counter and thus our strict API. But I suppose it's very rare case and we shouldn't take it into account. So let it be always global, and use simple API function to get resolution: odp_time_rate() or odp_time_res() if in ns.
> > I didn't see any word about global time in API, but also I didn't see > any word that > it has to be local. So I supposed that application should think that > it's worst case - local. > So, if it has to be global, it definitely should be described in the > API. > I only worry about that the striker requirement the less portability. Yes, currently it's not specified if time is global vs. local. We need to specify it. > > I've created the above API (including "sum" word as apposite to > "diff"). > Except global/local differentiation and resolution. > Also, I've added new definition that represents 0 ticks for time - > ODP_TIME_NULL. It can be used instead of odp_time_from_ns(0), and > is useful for comparison and initialization. > Also odp_time_to_u64() to get "real" ticks where it's needed. > > /** > * @def ODP_TIME_NULL > * Zero time handle > */ > > /** > * Current time handle of best hi-resolution time source available We have used term "handle" when an object is created/destroyed. Here odp_time_t is just an opaque value. User can store it infinitely (no need to destroy). So, I'd use just "time value", "time" or "time stamp" instead of handle. I would not promise "best hi-resolution time source available". A soc may have many time sources, but may not be able to use all of those for wall clock time (the "best one" is likely used for something else). "Current global time" is enough details. > * > * @return Time handle on success > * @retval ODP_TIME_INVALID on failure and errno set. > */ > odp_time_t odp_time(void); > > /** > * Time difference > * > * @param t1 First time stamp > * @param t2 Second time stamp > * > * @return Difference of time stamps in time handles > */ > odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); This needs to specify if timestamp order counts. I think it's better to count. If user don't know which one is the latter timestamp it can first compare with odp_time_cmp(). However, when user does know the order, the implementation can exploit that and save the cmp(). So, t2 >= t1 and function returns how much later t2 was stamped from t1. > > /** > * Time sum > * > * @param t1 First time stamp > * @param t2 Second time stamp > * > * @return Sum of time stamps in time handles > */ > odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2); Here order does not matter and it should be also documented. > > /** > * Convert time handle to nanoseconds > * > * @param time Time handle representing time > * > * @return Time in nanoseconds > */ > uint64_t odp_time_to_ns(odp_time_t time); // convert time to nsec // nsec value should start from 0 in ODP init (nsec value must not wrap in several years) // Global and local time may result different nsec values (different time) Nsec time should not wrap in the near future (e.g. for next 58 years @ 10GHz == 64 bits). So the nsec time value should be reseted to 0 in some point in ODP initialization. This is needs to be documented. > > /** > * Convert nanoseconds to time of best hi-resolution time source > available > * > * @param ns Time in nanoseconds > * > * @return Time handle representing time > */ > 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 t1 < t2, 0 if t1 = t2, 1 if t1 > t2 > */ > int odp_time_cmp(odp_time_t t1, odp_time_t t2); // Compare time values // -1: t2 < t1 // 0: t2 == t1 // 1: t2 > t1 Again, order matters (user tests the order here) and it's better to keep same order/spec throughout the API if (cpm(t1, t2) >= 0) { // normal case, t2 > t1 // foo expects that t2 >= t1 foo(t1, t2) } else { // not the normal case, t2 < t1 foo(t2, t1) } When order matter "t1" is specified to happen first in time and is left from t2 (in function parameters). > > /** > * Get printable value for an odp_time_t > * > * @param hdl odp_time_t handle to be printed > * @return uint64_t value that can be used to print/display this > * handle > * > * @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 handle. > */ > uint64_t odp_time_to_u64(odp_time_t hdl); Nsec is also printable, so not sure how much this is needed. Maybe for debugging conversion functions... > > According resolution. > I'm changing existent API, not create new one, so firstly I want to add > changes required after API change and then add resolution series that > will > add resolution API and correct places where it should be used. It's OK to first update everything to be "global". But leave room for the local spec. // global timestamp odp_time() // local timestamp odp_time_local() > > According global/local time. > As I see, there can be corner cases: > - no global timer, but it can be emulated by local > (start local timers synchronously or in another way) > - global timer but no local. In this case local timer can be emulated > by global. > > We can replace local on global, global on local. > If so we can always use global time source and don't bother with > local/global, > And if they have different resolution - choose the best and emulate if > needed. > I mean that main criteria is resolution. > If platform cannot emulate it for some reason, then we can have > platforms that > cannot implement global counter and thus our strict API. But I suppose > it's very > rare case and we shouldn't take it into account. > > So let it be always global, and use simple API function to get > resolution: > odp_time_rate() or odp_time_res() if in ns. As said before, implementation can implement "global time" with a cpu local counter (if that keeps in synch with all other cpus) or implement "local time" with a SoC global counter (if it's relatively fast to read it). Those are implementation details. Global vs. local time is very significant difference from application point of view. When application needs to compare timestamps from multiple threads (likely in a multi-threaded app), it needs global time no matter how many cycles it costs to get it. When application does thread local time stamping (e.g. timestamp different protocol stack stages, etc), it does not need guarantees that the time source is synchronized with other threads - it just needs as fast as possible access to a time source. When we keep local and global time sources separate, implementation can better optimize for both cases: - globally synchronous, but potentially high overhead - local and potentially fast -Petri
Petri, On 31.08.15 15:13, Savolainen, Petri (Nokia - FI/Espoo) wrote: > >> >> I didn't see any word about global time in API, but also I didn't see >> any word that >> it has to be local. So I supposed that application should think that >> it's worst case - local. >> So, if it has to be global, it definitely should be described in the >> API. >> I only worry about that the striker requirement the less portability. > > Yes, currently it's not specified if time is global vs. local. We need to specify it. Ok. > >> >> I've created the above API (including "sum" word as apposite to >> "diff"). >> Except global/local differentiation and resolution. >> Also, I've added new definition that represents 0 ticks for time - >> ODP_TIME_NULL. It can be used instead of odp_time_from_ns(0), and >> is useful for comparison and initialization. >> Also odp_time_to_u64() to get "real" ticks where it's needed. >> >> /** >> * @def ODP_TIME_NULL >> * Zero time handle >> */ >> >> /** >> * Current time handle of best hi-resolution time source available > > We have used term "handle" when an object is created/destroyed. Here odp_time_t is just an opaque value. User can store it infinitely (no need to destroy). So, I'd use just "time value", "time" or "time stamp" instead of handle. > > I would not promise "best hi-resolution time source available". A soc may have many time sources, but may not be able to use all of those for wall clock time (the "best one" is likely used for something else). > > "Current global time" is enough details. Ok. > >> * >> * @return Time handle on success >> * @retval ODP_TIME_INVALID on failure and errno set. >> */ >> odp_time_t odp_time(void); >> >> /** >> * Time difference >> * >> * @param t1 First time stamp >> * @param t2 Second time stamp >> * >> * @return Difference of time stamps in time handles >> */ >> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); > > This needs to specify if timestamp order counts. I think it's better to count. Yes, order counts. But I thinks it's obvious from t1 - First time stamp, t2 - Second time stamp, which was got later. > If user don't know which one is the latter timestamp it can first compare with odp_time_cmp(). However, when user does know the order, the implementation can exploit that and save the cmp(). User always knows order. And cmp() cannot be used for this, if t1 is less then t2 it doesn't mean that t1 < t2 for timestamps. The t2 can be got after overflow, and in absolute units (as range) it can be < t1, but in timestamps it will be > t1, and odp_time_diff takes in account it, and returns correct value. > > So, t2 >= t1 and function returns how much later t2 was stamped from t1. On my opinion it's correctly described in current version. No see reason to change it. * @param t1 First time stamp * @param t2 Second time stamp > >> >> /** >> * Time sum >> * >> * @param t1 First time stamp >> * @param t2 Second time stamp >> * >> * @return Sum of time stamps in time handles >> */ >> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2); > > Here order does not matter and it should be also documented. > Yep. I'll update. > >> >> /** >> * Convert time handle to nanoseconds >> * >> * @param time Time handle representing time >> * >> * @return Time in nanoseconds >> */ >> uint64_t odp_time_to_ns(odp_time_t time); > > > // convert time to nsec > // nsec value should start from 0 in ODP init (nsec value must not wrap in several years) > // Global and local time may result different nsec values (different time) > > Nsec time should not wrap in the near future (e.g. for next 58 years @ 10GHz == 64 bits). So the nsec time value should be reseted to 0 in some point in ODP initialization. This is needs to be documented. By default h/w counter is reseted at init. But we shouldn't guarantee it. Some SDK cannot allow to reset timer, only read, so if counter is assigned 0 at bootloader stage, it can be equal to ~ 0-10sec ~ 0-100000000000ns, when your application's started (it can be started any time). You can have two applications that started at different time....etc. So you cannot add such strict requirement, as in another case it requires an implementation to remember some init value X, and every time when odp_time() is called extract this X from current timestamp. This is not acceptable in cases when this is used to measure small times in s/w cycles. In most cases we need only relative time, cases when we need wall time can be counted on the fingers. For log function, if it's needed, we can remember init value X and extract it each time when print, but it can be done only for this use-case and others use-cases (which most) can be free from this redundancy. > >> >> /** >> * Convert nanoseconds to time of best hi-resolution time source >> available >> * >> * @param ns Time in nanoseconds >> * >> * @return Time handle representing time >> */ >> 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 t1 < t2, 0 if t1 = t2, 1 if t1 > t2 >> */ >> int odp_time_cmp(odp_time_t t1, odp_time_t t2); > > // Compare time values > // -1: t2 < t1 > // 0: t2 == t1 > // 1: t2 > t1 > > Again, order matters (user tests the order here) and it's better to keep same order/spec throughout the API No objection. I will correct. Initially I've taken this from strcmp...it's in backward order there. > > if (cpm(t1, t2) >= 0) { > // normal case, t2 > t1 > // foo expects that t2 >= t1 > foo(t1, t2) > } else { > // not the normal case, t2 < t1 > foo(t2, t1) > } > > > When order matter "t1" is specified to happen first in time and is left from t2 (in function parameters). > > >> >> /** >> * Get printable value for an odp_time_t >> * >> * @param hdl odp_time_t handle to be printed >> * @return uint64_t value that can be used to print/display this >> * handle >> * >> * @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 handle. >> */ >> uint64_t odp_time_to_u64(odp_time_t hdl); > > Nsec is also printable, so not sure how much this is needed. Maybe for debugging conversion functions... It's widely used in odp tests/examples for debugg purposes. W/o this call I cannot port new API correctly. > >> >> According resolution. >> I'm changing existent API, not create new one, so firstly I want to add >> changes required after API change and then add resolution series that >> will >> add resolution API and correct places where it should be used. > > It's OK to first update everything to be "global". But leave room for the local spec. I'm not going to add local time, at least in this series. Reasons: 1 - it increases complexity. 2 - it requires to hold different types of time under opaque type, thus it requires each time check the type of time under odp_time(), which can be used in places sensible for that. 3 - if local counters have better characteristics they can emulate global timer, in turn global timer can be used everywhere. 4 - if it will be required, it can be freely added as additional call, w/o modification existent API > > // global timestamp > odp_time() > > // local timestamp > odp_time_local() > >> >> According global/local time. >> As I see, there can be corner cases: >> - no global timer, but it can be emulated by local >> (start local timers synchronously or in another way) >> - global timer but no local. In this case local timer can be emulated >> by global. >> >> We can replace local on global, global on local. >> If so we can always use global time source and don't bother with >> local/global, >> And if they have different resolution - choose the best and emulate if >> needed. >> I mean that main criteria is resolution. >> If platform cannot emulate it for some reason, then we can have >> platforms that >> cannot implement global counter and thus our strict API. But I suppose >> it's very >> rare case and we shouldn't take it into account. >> >> So let it be always global, and use simple API function to get >> resolution: >> odp_time_rate() or odp_time_res() if in ns. > > As said before, implementation can implement "global time" with a cpu local counter (if that keeps in synch with all other cpus) or implement "local time" with a SoC global counter (if it's relatively fast to read it). Those are implementation details. > > Global vs. local time is very significant difference from application point of view. When application needs to compare timestamps from multiple threads (likely in a multi-threaded app), it needs global time no matter how many cycles it costs to get it. When application does thread local time stamping (e.g. timestamp different protocol stack stages, etc), it does not need guarantees that the time source is synchronized with other threads - it just needs as fast as possible access to a time source. > > When we keep local and global time sources separate, implementation can better optimize for both cases: > - globally synchronous, but potentially high overhead > - local and potentially fast > > > -Petri > >
> >> * > >> * @return Time handle on success > >> * @retval ODP_TIME_INVALID on failure and errno set. > >> */ > >> odp_time_t odp_time(void); > >> > >> /** > >> * Time difference > >> * > >> * @param t1 First time stamp > >> * @param t2 Second time stamp > >> * > >> * @return Difference of time stamps in time handles > >> */ > >> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); > > > > This needs to specify if timestamp order counts. I think it's better > to count. > > Yes, order counts. But I thinks it's obvious from > t1 - First time stamp, > t2 - Second time stamp, which was got later. > > > If user don't know which one is the latter timestamp it can first > compare with odp_time_cmp(). However, when user does know the order, > the implementation can exploit that and save the cmp(). > > User always knows order. Typically user knows order for local time stamps, but likely does not know for global timestamps from two different threads. > And cmp() cannot be used for this, if t1 is less then t2 it doesn't > mean that t1 < t2 for timestamps. > The t2 can be got after overflow, and in absolute units (as range) it > can be < t1, but in timestamps > it will be > t1, and odp_time_diff takes in account it, and returns > correct value. Implementation decides what information it carries in odp_time_t and how it handles possible time stamp counter wraps. The compare function returns relative order of t1 and t2 in time (not in counter value). On single thread: t1 = odp_time(); t2 = odp_time(); ret = odp_time_cmp(t1, t2); "ret" is never -1 (always 0 or 1), since t2 stamp cannot be taken before t1 in time. The implementation internal counter may have wrapped, but implementation takes care of that. > > > > > So, t2 >= t1 and function returns how much later t2 was stamped from > t1. > > On my opinion it's correctly described in current version. No see > reason to change it. > * @param t1 First time stamp > * @param t2 Second time stamp It's half way there, but should be very clear that t1 is always stamped before t2 (or at the same time, never after t2). > > > > >> > >> /** > >> * Time sum > >> * > >> * @param t1 First time stamp > >> * @param t2 Second time stamp > >> * > >> * @return Sum of time stamps in time handles > >> */ > >> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2); > > > > Here order does not matter and it should be also documented. > > > > Yep. I'll update. > > > > >> > >> /** > >> * Convert time handle to nanoseconds > >> * > >> * @param time Time handle representing time > >> * > >> * @return Time in nanoseconds > >> */ > >> uint64_t odp_time_to_ns(odp_time_t time); > > > > > > // convert time to nsec > > // nsec value should start from 0 in ODP init (nsec value must not > wrap in several years) > > // Global and local time may result different nsec values (different > time) > > > > Nsec time should not wrap in the near future (e.g. for next 58 years > @ 10GHz == 64 bits). So the nsec time value should be reseted to 0 in > some point in ODP initialization. This is needs to be documented. > > By default h/w counter is reseted at init. But we shouldn't guarantee > it. > Some SDK cannot allow to reset timer, only read, so > if counter is assigned 0 at bootloader stage, it can be equal to ~ 0- > 10sec ~ 0-100000000000ns, > when your application's started (it can be started any time). You can > have two applications > that started at different time....etc. > > So you cannot add such strict requirement, as in another case it > requires an implementation to remember > some init value X, and every time when odp_time() is called extract > this X from current timestamp. > This is not acceptable in cases when this is used to measure small > times in s/w cycles. In most cases > we need only relative time, cases when we need wall time can be counted > on the fingers. > For log function, if it's needed, we can remember init value X and > extract it each time when print, > but it can be done only for this use-case and others use-cases (which > most) can be free from this redundancy. odp_time_t is there to give freedom for implementation to use whatever counter, not require to reset it, etc. Only the wall clock time in nsec (msec, sec, ...) must not wrap. We need to spec it so that only the conversion function would do the extraction. // 64 bit time counter value at start 0xdead0000 // counter runs at 1GHz odp_init_global(); // store counter value = 100 000 + 0xdead0000 t1 = odp_time(); // store counter value = 101 000 + 0xdead0000 t2 = odp_time(); // (100 000 + 0xdead0000) - 0xdead0000 // return 100 000 nsec nsec1 = odp_time_to_ns(t1); // (101 000 + 0xdead0000) - 0xdead0000 // return 101 000 nsec nsec2 = odp_time_to_ns(t1); // diff = (101 000 + 0xdead0000) - (100 000 + 0xdead0000) = 1000 // store 1 000 + 0xdead0000 t3 = odp_time_diff(t1, t2); // (1 000 + 0xdead0000) - 0xdead0000 // return 1 000 nsec nsec3 = odp_time_to_ns(t3); > > > > >> > >> /** > >> * Convert nanoseconds to time of best hi-resolution time source > >> available > >> * > >> * @param ns Time in nanoseconds > >> * > >> * @return Time handle representing time > >> */ > >> 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 t1 < t2, 0 if t1 = t2, 1 if t1 > t2 > >> */ > >> int odp_time_cmp(odp_time_t t1, odp_time_t t2); > > > > // Compare time values > > // -1: t2 < t1 > > // 0: t2 == t1 > > // 1: t2 > t1 > > > > Again, order matters (user tests the order here) and it's better to > keep same order/spec throughout the API > > No objection. I will correct. > Initially I've taken this from strcmp...it's in backward order there. Yes, looked also strcmp but this order is more logical here. > > > > > if (cpm(t1, t2) >= 0) { > > // normal case, t2 > t1 > > // foo expects that t2 >= t1 > > foo(t1, t2) > > } else { > > // not the normal case, t2 < t1 > > foo(t2, t1) > > } > > > > > > When order matter "t1" is specified to happen first in time and is > left from t2 (in function parameters). > > > > > >> > >> /** > >> * Get printable value for an odp_time_t > >> * > >> * @param hdl odp_time_t handle to be printed > >> * @return uint64_t value that can be used to print/display > this > >> * handle > >> * > >> * @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 handle. > >> */ > >> uint64_t odp_time_to_u64(odp_time_t hdl); > > > > Nsec is also printable, so not sure how much this is needed. Maybe > for debugging conversion functions... > > It's widely used in odp tests/examples for debugg purposes. > W/o this call I cannot port new API correctly. In general, odp_time_t can be a struct larger than 64 bits. For example, typedef struct { uint8_t flags; // global/local time, wrapped/not wrapped, etc ... uint64_t hw_counter_value; } > > > > >> > >> According resolution. > >> I'm changing existent API, not create new one, so firstly I want to > add > >> changes required after API change and then add resolution series > that > >> will > >> add resolution API and correct places where it should be used. > > > > It's OK to first update everything to be "global". But leave room for > the local spec. > > I'm not going to add local time, at least in this series. > Reasons: > 1 - it increases complexity. > 2 - it requires to hold different types of time under opaque type, thus > it > requires each time check the type of time under odp_time(), which > can be used in > places sensible for that. > 3 - if local counters have better characteristics they can emulate > global timer, > in turn global timer can be used everywhere. > 4 - if it will be required, it can be freely added as additional call, > w/o modification existent API It's OK to add local time in next step. However, applications need another round of changes since many of those may not actually need global time (timestamp under single thread) and could run faster on a platform that can optimize local but cannot optimize global time. Local implementation can be the same with global, which would not add complexity or checks. -Petri
Hi, Petri On 01.09.15 11:33, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > >>>> * >>>> * @return Time handle on success >>>> * @retval ODP_TIME_INVALID on failure and errno set. >>>> */ >>>> odp_time_t odp_time(void); >>>> >>>> /** >>>> * Time difference >>>> * >>>> * @param t1 First time stamp >>>> * @param t2 Second time stamp >>>> * >>>> * @return Difference of time stamps in time handles >>>> */ >>>> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); >>> >>> This needs to specify if timestamp order counts. I think it's better >> to count. >> >> Yes, order counts. But I thinks it's obvious from >> t1 - First time stamp, >> t2 - Second time stamp, which was got later. >> >>> If user don't know which one is the latter timestamp it can first >> compare with odp_time_cmp(). However, when user does know the order, >> the implementation can exploit that and save the cmp(). >> >> User always knows order. > > Typically user knows order for local time stamps, but likely does not know for global timestamps from two different threads. But user cannot check order with cmp, see below. Thus he must know order. > > >> And cmp() cannot be used for this, if t1 is less then t2 it doesn't >> mean that t1 < t2 for timestamps. >> The t2 can be got after overflow, and in absolute units (as range) it >> can be < t1, but in timestamps >> it will be > t1, and odp_time_diff takes in account it, and returns >> correct value. > > Implementation decides what information it carries in odp_time_t and how it handles possible time stamp counter wraps. The compare function returns relative order of t1 and t2 in time (not in counter value). > > On single thread: > t1 = odp_time(); > t2 = odp_time(); > > ret = odp_time_cmp(t1, t2); > > "ret" is never -1 (always 0 or 1), since t2 stamp cannot be taken before t1 in time. The implementation internal counter may have wrapped, but implementation takes care of that. > Currently if you look in the sources everywhere we compare ranges but not wall time. That is because counter can overflow. Typical usage is: wait_time = 23; start_time = odp_time(); do { time = odp_time(); diff = odp_time_diff(start_time, time); } while (odp_time_cmp(diff, wait_time) > 0); The odp_time_diff() is used here only because time counter can overflow, and diff function takes care of that. No any ns here. It's current implementation, odp_time_cmp() is used only to compare time ranges, not wall times. Just instead of odp_time_cmp() examples/tests use simple compare like while (range1 < range2). After adding opaque type we must use odp_time_cmp for that, that's only difference. This series is intended to unbind CPU cycle from time API, not change time API at once to be wall time. If we can guarantee time count cannot overflow, than we don't need odp_time_cmp at all, we can use diff for that. But currently, we cannot guarantee it. In this series odp_time_t opaque type is added only with one main intention: ask application to use odp_time_diff() and not compare times directly. In another case all this API is not needed at all, it can be simplified to simple functions: odp_time(); odp_time_ns(uint64 time_stamp); odp_time_res(); That's all API. If time cannot overflow, then why diff?, Do simply t2 - t1 and live easily. As time never overflows, than we always can compare t1 and t2 w/o problems, why not, t1 < t2, it's simple and fast, faster cannot be. Need ns, use above great API function odp_time_ns(). Need to add local time? add additional one API, why not? odp_time_local_ns()..... Why this stuff is needed at all? It was added because time counter can overflow. How your implementation is going to control that? When you read next time, how do you know that it's overflowed?, remember specifically previous state, run additional kernel to read it periodically? If no - then use current API or don't use it at all. Why do we need to overload simple functions, aim of which simply measure short periods of time? check time type in each of them, extract some init values, control overflow...etc. >> >>> >>> So, t2 >= t1 and function returns how much later t2 was stamped from >> t1. >> >> On my opinion it's correctly described in current version. No see >> reason to change it. >> * @param t1 First time stamp >> * @param t2 Second time stamp > > > It's half way there, but should be very clear that t1 is always stamped before t2 (or at the same time, never after t2). It's not correct to hardly write it here. It can be used to diff time ranges, not only timestamps. Probably better simply name it: >> * @param t1 First time >> * @param t2 Second time and add some line like: if diff timestamps (most cases) t2 is supposed to be got after t1. > > >> >>> >>>> >>>> /** >>>> * Time sum >>>> * >>>> * @param t1 First time stamp >>>> * @param t2 Second time stamp >>>> * >>>> * @return Sum of time stamps in time handles >>>> */ >>>> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2); >>> >>> Here order does not matter and it should be also documented. >>> >> >> Yep. I'll update. >> >>> >>>> >>>> /** >>>> * Convert time handle to nanoseconds >>>> * >>>> * @param time Time handle representing time >>>> * >>>> * @return Time in nanoseconds >>>> */ >>>> uint64_t odp_time_to_ns(odp_time_t time); >>> >>> >>> // convert time to nsec >>> // nsec value should start from 0 in ODP init (nsec value must not >> wrap in several years) >>> // Global and local time may result different nsec values (different >> time) >>> >>> Nsec time should not wrap in the near future (e.g. for next 58 years >> @ 10GHz == 64 bits). So the nsec time value should be reseted to 0 in >> some point in ODP initialization. This is needs to be documented. >> >> By default h/w counter is reseted at init. But we shouldn't guarantee >> it. >> Some SDK cannot allow to reset timer, only read, so >> if counter is assigned 0 at bootloader stage, it can be equal to ~ 0- >> 10sec ~ 0-100000000000ns, >> when your application's started (it can be started any time). You can >> have two applications >> that started at different time....etc. >> >> So you cannot add such strict requirement, as in another case it >> requires an implementation to remember >> some init value X, and every time when odp_time() is called extract >> this X from current timestamp. >> This is not acceptable in cases when this is used to measure small >> times in s/w cycles. In most cases >> we need only relative time, cases when we need wall time can be counted >> on the fingers. >> For log function, if it's needed, we can remember init value X and >> extract it each time when print, >> but it can be done only for this use-case and others use-cases (which >> most) can be free from this redundancy. > > odp_time_t is there to give freedom for implementation to use whatever counter, not require to reset it, etc. > Only the wall clock time in nsec (msec, sec, ...) must not wrap. We need to spec it so that only the conversion function would do the extraction. > > // 64 bit time counter value at start 0xdead0000 > // counter runs at 1GHz > odp_init_global(); > > // store counter value = 100 000 + 0xdead0000 > t1 = odp_time(); > > // store counter value = 101 000 + 0xdead0000 > t2 = odp_time(); > > // (100 000 + 0xdead0000) - 0xdead0000 > // return 100 000 nsec You meant return 100 000/rate; > nsec1 = odp_time_to_ns(t1); > > // (101 000 + 0xdead0000) - 0xdead0000 > // return 101 000 nsec Again, about overflows, it be a little harder in implementation. what if time value were overflowed? Say you current time_stamp X is < 200? X - 0xdead0000 = 0xbaddead0 Even if not less than 0xdead0000, it will extract, but in fact it will be incorrect. It should be rewritten like: return (odp_time_diff(0xdead0000 - odp_time()))/rate; That's why better to use this function to convert ranges, that is relative time. Say you need print time in log: remember in odp_init_global(); start_time = odp_time(); then in print function: print ("%ll bla-bla-bla", odp_time_ns(odp_time_diff(start_time - odp_time()))); and no need to worry about wall time and overlaps and additional actions in other places where it's not needed. > nsec2 = odp_time_to_ns(t1); > > // diff = (101 000 + 0xdead0000) - (100 000 + 0xdead0000) = 1000 > // store 1 000 + 0xdead0000 That's the moment I'm worrying about. Each time you need to do + in some cycle. Thanks that it's not in odp_time(), it makes me worry a little less. > t3 = odp_time_diff(t1, t2); > > // (1 000 + 0xdead0000) - 0xdead0000 > // return 1 000 nsec > nsec3 = odp_time_to_ns(t3); > > > > >> >>> >>>> >>>> /** >>>> * Convert nanoseconds to time of best hi-resolution time source >>>> available >>>> * >>>> * @param ns Time in nanoseconds >>>> * >>>> * @return Time handle representing time >>>> */ >>>> 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 t1 < t2, 0 if t1 = t2, 1 if t1 > t2 >>>> */ >>>> int odp_time_cmp(odp_time_t t1, odp_time_t t2); >>> >>> // Compare time values >>> // -1: t2 < t1 >>> // 0: t2 == t1 >>> // 1: t2 > t1 >>> >>> Again, order matters (user tests the order here) and it's better to >> keep same order/spec throughout the API >> >> No objection. I will correct. >> Initially I've taken this from strcmp...it's in backward order there. > > Yes, looked also strcmp but this order is more logical here. > > >> >>> >>> if (cpm(t1, t2) >= 0) { >>> // normal case, t2 > t1 >>> // foo expects that t2 >= t1 >>> foo(t1, t2) >>> } else { >>> // not the normal case, t2 < t1 >>> foo(t2, t1) >>> } >>> >>> >>> When order matter "t1" is specified to happen first in time and is >> left from t2 (in function parameters). >>> >>> >>>> >>>> /** >>>> * Get printable value for an odp_time_t >>>> * >>>> * @param hdl odp_time_t handle to be printed >>>> * @return uint64_t value that can be used to print/display >> this >>>> * handle >>>> * >>>> * @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 handle. >>>> */ >>>> uint64_t odp_time_to_u64(odp_time_t hdl); >>> >>> Nsec is also printable, so not sure how much this is needed. Maybe >> for debugging conversion functions... >> >> It's widely used in odp tests/examples for debugg purposes. >> W/o this call I cannot port new API correctly. > > > In general, odp_time_t can be a struct larger than 64 bits. For example, > > typedef struct { > uint8_t flags; // global/local time, wrapped/not wrapped, etc ... > uint64_t hw_counter_value; > } Then here we just return hw_counter_value. User who use this function should know what he is doing. > > > >> >>> >>>> >>>> According resolution. >>>> I'm changing existent API, not create new one, so firstly I want to >> add >>>> changes required after API change and then add resolution series >> that >>>> will >>>> add resolution API and correct places where it should be used. >>> >>> It's OK to first update everything to be "global". But leave room for >> the local spec. >> >> I'm not going to add local time, at least in this series. >> Reasons: >> 1 - it increases complexity. >> 2 - it requires to hold different types of time under opaque type, thus >> it >> requires each time check the type of time under odp_time(), which >> can be used in >> places sensible for that. >> 3 - if local counters have better characteristics they can emulate >> global timer, >> in turn global timer can be used everywhere. >> 4 - if it will be required, it can be freely added as additional call, >> w/o modification existent API > > It's OK to add local time in next step. Ok. > However, applications need another round of changes since many of those may not actually need global time (timestamp under single thread) and could run faster on a platform that can optimize local but cannot optimize global time. Let allow time to check it. If it will be needed we can add it. > > Local implementation can be the same with global, which would not add complexity or checks. > > > > -Petri > >
> -----Original Message----- > From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] > Sent: Tuesday, September 01, 2015 2:20 PM > To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; > Stuart Haslam; Maxim Uvarov > Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU > cycles from time API > > Hi, Petri > > On 01.09.15 11:33, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > > > > >>>> * > >>>> * @return Time handle on success > >>>> * @retval ODP_TIME_INVALID on failure and errno set. > >>>> */ > >>>> odp_time_t odp_time(void); > >>>> > >>>> /** > >>>> * Time difference > >>>> * > >>>> * @param t1 First time stamp > >>>> * @param t2 Second time stamp > >>>> * > >>>> * @return Difference of time stamps in time handles > >>>> */ > >>>> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); > >>> > >>> This needs to specify if timestamp order counts. I think it's > better > >> to count. > >> > >> Yes, order counts. But I thinks it's obvious from > >> t1 - First time stamp, > >> t2 - Second time stamp, which was got later. > >> > >>> If user don't know which one is the latter timestamp it can first > >> compare with odp_time_cmp(). However, when user does know the order, > >> the implementation can exploit that and save the cmp(). > >> > >> User always knows order. > > > > Typically user knows order for local time stamps, but likely does not > know for global timestamps from two different threads. > > But user cannot check order with cmp, see below. > Thus he must know order. E.g. two threads time stamping incoming messages. After a while a thread wants to compare which one was received first. Need global wall time and an API to compare those. > > > > > > >> And cmp() cannot be used for this, if t1 is less then t2 it doesn't > >> mean that t1 < t2 for timestamps. > >> The t2 can be got after overflow, and in absolute units (as range) > it > >> can be < t1, but in timestamps > >> it will be > t1, and odp_time_diff takes in account it, and returns > >> correct value. > > > > Implementation decides what information it carries in odp_time_t and > how it handles possible time stamp counter wraps. The compare function > returns relative order of t1 and t2 in time (not in counter value). > > > > On single thread: > > t1 = odp_time(); > > t2 = odp_time(); > > > > ret = odp_time_cmp(t1, t2); > > > > "ret" is never -1 (always 0 or 1), since t2 stamp cannot be taken > before t1 in time. The implementation internal counter may have > wrapped, but implementation takes care of that. > > > > Currently if you look in the sources everywhere we compare ranges but > not wall time. Examples are not representing entire range of data plane apps. Both absolute wall time and difference in wall time are valid use cases. > That is because counter can overflow. > Typical usage is: > > wait_time = 23; > start_time = odp_time(); > do { > time = odp_time(); > diff = odp_time_diff(start_time, time); > } while (odp_time_cmp(diff, wait_time) > 0); // Spin waiting for 23 nsec. A timer would be better for this...but here we go wait_time = odp_time_from_ns(23); cur_time = odp_time(); stop_time = odp_time_sum(cur_time, wait_time); do { time = odp_time(); } while (odp_time_cmp(time, stop_time) > 0); Yes, originally diff and direct cycle count compare was used. Now the wait loop can just compare current time against the stop time. > > The odp_time_diff() is used here only because time counter can > overflow, and diff function takes care of that. > No any ns here. Wait time should be 23 nsec. No CPU cycles any more. > It's current implementation, odp_time_cmp() is used only to compare > time ranges, not wall times. > Just instead of odp_time_cmp() examples/tests use simple compare like > while (range1 < range2). > After adding opaque type we must use odp_time_cmp for that, that's only > difference. > > This series is intended to unbind CPU cycle from time API, not change > time API at once to be wall time. > If we can guarantee time count cannot overflow, than we don't need > odp_time_cmp at all, we can use diff for that. > But currently, we cannot guarantee it. In this series odp_time_t opaque > type is added only with one main intention: > ask application to use odp_time_diff() and not compare times directly. Yes, agree. Still any API change should be forward compatible to global vs. local time, and absolute time vs. range. > > In another case all this API is not needed at all, it can be simplified > to simple functions: > > odp_time(); > odp_time_ns(uint64 time_stamp); > odp_time_res(); > > That's all API. If time cannot overflow, then why diff?, Do simply t2 - > t1 and live easily. > As time never overflows, than we always can compare t1 and t2 w/o > problems, why not, t1 < t2, > it's simple and fast, faster cannot be. Need ns, use above great API > function odp_time_ns(). > Need to add local time? add additional one API, why not? > odp_time_local_ns()..... > > Why this stuff is needed at all? It was added because time counter can > overflow. > How your implementation is going to control that? When you read next > time, how do you know that > it's overflowed?, remember specifically previous state, run additional > kernel to read it periodically? > If no - then use current API or don't use it at all. Why do we need to > overload simple functions, aim of > which simply measure short periods of time? check time type in each of > them, extract some init values, > control overflow...etc. For sure the HW counter can overflow (32 bit counters or counter not init to zero). Implementation can easily detect one wrap around and that's typically enough. A 32 bit counter wraps in 4 seconds at 1 GHz. It can be an implementation configuration time trade off to select high resolution vs. long wrap around time (longest possible interval to measure). Global and local time can have different config or limitations (documented in release notes). > > >> > >>> > >>> So, t2 >= t1 and function returns how much later t2 was stamped > from > >> t1. > >> > >> On my opinion it's correctly described in current version. No see > >> reason to change it. > >> * @param t1 First time stamp > >> * @param t2 Second time stamp > > > > > > It's half way there, but should be very clear that t1 is always > stamped before t2 (or at the same time, never after t2). > > It's not correct to hardly write it here. > It can be used to diff time ranges, not only timestamps. > Probably better simply name it: > >> * @param t1 First time > >> * @param t2 Second time > > and add some line like: if diff timestamps (most cases) t2 is supposed > to be got after t1. > > > > > > >> > >>> > >>>> > >>>> /** > >>>> * Time sum > >>>> * > >>>> * @param t1 First time stamp > >>>> * @param t2 Second time stamp > >>>> * > >>>> * @return Sum of time stamps in time handles > >>>> */ > >>>> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2); > >>> > >>> Here order does not matter and it should be also documented. > >>> > >> > >> Yep. I'll update. > >> > >>> > >>>> > >>>> /** > >>>> * Convert time handle to nanoseconds > >>>> * > >>>> * @param time Time handle representing time > >>>> * > >>>> * @return Time in nanoseconds > >>>> */ > >>>> uint64_t odp_time_to_ns(odp_time_t time); > >>> > >>> > >>> // convert time to nsec > >>> // nsec value should start from 0 in ODP init (nsec value must not > >> wrap in several years) > >>> // Global and local time may result different nsec values > (different > >> time) > >>> > >>> Nsec time should not wrap in the near future (e.g. for next 58 > years > >> @ 10GHz == 64 bits). So the nsec time value should be reseted to 0 > in > >> some point in ODP initialization. This is needs to be documented. > >> > >> By default h/w counter is reseted at init. But we shouldn't > guarantee > >> it. > >> Some SDK cannot allow to reset timer, only read, so > >> if counter is assigned 0 at bootloader stage, it can be equal to ~ > 0- > >> 10sec ~ 0-100000000000ns, > >> when your application's started (it can be started any time). You > can > >> have two applications > >> that started at different time....etc. > >> > >> So you cannot add such strict requirement, as in another case it > >> requires an implementation to remember > >> some init value X, and every time when odp_time() is called extract > >> this X from current timestamp. > >> This is not acceptable in cases when this is used to measure small > >> times in s/w cycles. In most cases > >> we need only relative time, cases when we need wall time can be > counted > >> on the fingers. > >> For log function, if it's needed, we can remember init value X and > >> extract it each time when print, > >> but it can be done only for this use-case and others use-cases > (which > >> most) can be free from this redundancy. > > > > odp_time_t is there to give freedom for implementation to use > whatever counter, not require to reset it, etc. > > Only the wall clock time in nsec (msec, sec, ...) must not wrap. We > need to spec it so that only the conversion function would do the > extraction. > > > > // 64 bit time counter value at start 0xdead0000 > > // counter runs at 1GHz > > odp_init_global(); > > > > // store counter value = 100 000 + 0xdead0000 > > t1 = odp_time(); > > > > // store counter value = 101 000 + 0xdead0000 > > t2 = odp_time(); > > > > // (100 000 + 0xdead0000) - 0xdead0000 > > // return 100 000 nsec > > You meant return 100 000/rate; Yes, in this simple case counter runs at 1 GHz: 1 cycles = 1 nsec. > > > nsec1 = odp_time_to_ns(t1); > > > > // (101 000 + 0xdead0000) - 0xdead0000 > > // return 101 000 nsec > > Again, about overflows, it be a little harder in implementation. > > what if time value were overflowed? > Say you current time_stamp X is < 200? > > X - 0xdead0000 = 0xbaddead0 > > Even if not less than 0xdead0000, it will extract, but in fact it will > be incorrect. > It should be rewritten like: > return (odp_time_diff(0xdead0000 - odp_time()))/rate; 1 - 0xdead0000 = 0x2154000 nsec, which is distance from 0xdead0000 to 1 at 1 GHz. The simple math can handle one wrap around. It's up to the implementation how many wraps it supports and on which resolution. I think one wrap is OK, but needs to be clearly documented. On 64 bit systems, one wrap is all you need. The next wrap happens e.g. in 580 year... > > That's why better to use this function to convert ranges, that is > relative time. > > Say you need print time in log: > remember in odp_init_global(); > start_time = odp_time(); > > then in print function: > > print ("%ll bla-bla-bla", odp_time_ns(odp_time_diff(start_time - > odp_time()))); > and no need to worry about wall time and overlaps and additional > actions in other places > where it's not needed. Global time and wraps is clearly better to solve once and in the implementation, instead of everyone solving it separately. It's just matter of correct API definition to support all use cases with good performance. > > > nsec2 = odp_time_to_ns(t1); > > > > // diff = (101 000 + 0xdead0000) - (100 000 + 0xdead0000) = 1000 > > // store 1 000 + 0xdead0000 > > That's the moment I'm worrying about. > Each time you need to do + in some cycle. > Thanks that it's not in odp_time(), it makes me worry a little less. Since it's stored in to odp_time_t, you can also store 1000 and a flag telling that it's a range instead of an absolute value. It's up to implementation. We could have separate set of function calls for ranges and absolute values, but does it pay off. I think the counter read is taking most of the cycles anyway, at least when it's not local to CPU. The rate division may also take some cycles. Simple add/sub operations are fast when the (constant) data is already in cache. > > > t3 = odp_time_diff(t1, t2); > > > > // (1 000 + 0xdead0000) - 0xdead0000 > > // return 1 000 nsec > > nsec3 = odp_time_to_ns(t3); > > > > > > > > > >> > >>> > >>>> > >>>> /** > >>>> * Convert nanoseconds to time of best hi-resolution time > source > >>>> available > >>>> * > >>>> * @param ns Time in nanoseconds > >>>> * > >>>> * @return Time handle representing time > >>>> */ > >>>> 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 t1 < t2, 0 if t1 = t2, 1 if t1 > t2 > >>>> */ > >>>> int odp_time_cmp(odp_time_t t1, odp_time_t t2); > >>> > >>> // Compare time values > >>> // -1: t2 < t1 > >>> // 0: t2 == t1 > >>> // 1: t2 > t1 > >>> > >>> Again, order matters (user tests the order here) and it's better to > >> keep same order/spec throughout the API > >> > >> No objection. I will correct. > >> Initially I've taken this from strcmp...it's in backward order > there. > > > > Yes, looked also strcmp but this order is more logical here. > > > > > >> > >>> > >>> if (cpm(t1, t2) >= 0) { > >>> // normal case, t2 > t1 > >>> // foo expects that t2 >= t1 > >>> foo(t1, t2) > >>> } else { > >>> // not the normal case, t2 < t1 > >>> foo(t2, t1) > >>> } > >>> > >>> > >>> When order matter "t1" is specified to happen first in time and is > >> left from t2 (in function parameters). > >>> > >>> > >>>> > >>>> /** > >>>> * Get printable value for an odp_time_t > >>>> * > >>>> * @param hdl odp_time_t handle to be printed > >>>> * @return uint64_t value that can be used to print/display > >> this > >>>> * handle > >>>> * > >>>> * @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 handle. > >>>> */ > >>>> uint64_t odp_time_to_u64(odp_time_t hdl); > >>> > >>> Nsec is also printable, so not sure how much this is needed. Maybe > >> for debugging conversion functions... > >> > >> It's widely used in odp tests/examples for debugg purposes. > >> W/o this call I cannot port new API correctly. > > > > > > In general, odp_time_t can be a struct larger than 64 bits. For > example, > > > > typedef struct { > > uint8_t flags; // global/local time, wrapped/not wrapped, etc ... > > uint64_t hw_counter_value; > > } > > Then here we just return hw_counter_value. User who use this function > should > know what he is doing. Just saying that these 64 bits are for debugging. If the counter value is already 64 bits, you need to leave out some data. The bug may be hiding in a corrupted flag bit that was not output ... -Petri
Petri, On 01.09.15 16:48, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -----Original Message----- >> From: ext Ivan Khoronzhuk [mailto:ivan.khoronzhuk@linaro.org] >> Sent: Tuesday, September 01, 2015 2:20 PM >> To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org; >> Stuart Haslam; Maxim Uvarov >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU >> cycles from time API >> >> Hi, Petri >> >> On 01.09.15 11:33, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>> >>> >>> >>>>>> * >>>>>> * @return Time handle on success >>>>>> * @retval ODP_TIME_INVALID on failure and errno set. >>>>>> */ >>>>>> odp_time_t odp_time(void); >>>>>> >>>>>> /** >>>>>> * Time difference >>>>>> * >>>>>> * @param t1 First time stamp >>>>>> * @param t2 Second time stamp >>>>>> * >>>>>> * @return Difference of time stamps in time handles >>>>>> */ >>>>>> odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2); >>>>> >>>>> This needs to specify if timestamp order counts. I think it's >> better >>>> to count. >>>> >>>> Yes, order counts. But I thinks it's obvious from >>>> t1 - First time stamp, >>>> t2 - Second time stamp, which was got later. >>>> >>>>> If user don't know which one is the latter timestamp it can first >>>> compare with odp_time_cmp(). However, when user does know the order, >>>> the implementation can exploit that and save the cmp(). >>>> >>>> User always knows order. >>> >>> Typically user knows order for local time stamps, but likely does not >> know for global timestamps from two different threads. >> >> But user cannot check order with cmp, see below. >> Thus he must know order. > > E.g. two threads time stamping incoming messages. After a while a thread wants to compare which one was received first. Need global wall time and an API to compare those. Use wall time and compare it in ns. But don't use functions that not intended for it - odp_time_cmp. > >> >>> >>> >>>> And cmp() cannot be used for this, if t1 is less then t2 it doesn't >>>> mean that t1 < t2 for timestamps. >>>> The t2 can be got after overflow, and in absolute units (as range) >> it >>>> can be < t1, but in timestamps >>>> it will be > t1, and odp_time_diff takes in account it, and returns >>>> correct value. >>> >>> Implementation decides what information it carries in odp_time_t and >> how it handles possible time stamp counter wraps. The compare function >> returns relative order of t1 and t2 in time (not in counter value). >>> >>> On single thread: >>> t1 = odp_time(); >>> t2 = odp_time(); >>> >>> ret = odp_time_cmp(t1, t2); >>> >>> "ret" is never -1 (always 0 or 1), since t2 stamp cannot be taken >> before t1 in time. The implementation internal counter may have >> wrapped, but implementation takes care of that. >>> >> >> Currently if you look in the sources everywhere we compare ranges but >> not wall time. > > Examples are not representing entire range of data plane apps. Examples and tests. Even validation test doesn't test on wall time, it checks only ranges. And I know why, because it was not supposed to be wall time. > Both absolute wall time and difference in wall time are valid use cases. For wall time we need to use separate functions probably, Current functions are not for this purpose. They can be implemented in help library based on current API, or added here with it's own functions, but not included. > > >> That is because counter can overflow. >> Typical usage is: >> >> wait_time = 23; >> start_time = odp_time(); >> do { >> time = odp_time(); >> diff = odp_time_diff(start_time, time); >> } while (odp_time_cmp(diff, wait_time) > 0); > > > > // Spin waiting for 23 nsec. A timer would be better for this...but here we go What would be better for this is no matter here, 23 nsec can be easily used in loop, depends on usecase. But I meant concrete 23 ticks of counter, ns also doesn't break idea. > wait_time = odp_time_from_ns(23); > cur_time = odp_time(); > stop_time = odp_time_sum(cur_time, wait_time); > > do { > time = odp_time(); > } while (odp_time_cmp(time, stop_time) > 0); > > That is example how we shouldn't do and avoid whenever possible. If it will be possible you don't need cmp and even diff functions. > Yes, originally diff and direct cycle count compare was used. > Now the wait loop can just compare current time against the stop time. No, it cannot. I've explained it several times why. > >> >> The odp_time_diff() is used here only because time counter can >> overflow, and diff function takes care of that. >> No any ns here. > > Wait time should be 23 nsec. No CPU cycles any more. > >> It's current implementation, odp_time_cmp() is used only to compare >> time ranges, not wall times. >> Just instead of odp_time_cmp() examples/tests use simple compare like >> while (range1 < range2). >> After adding opaque type we must use odp_time_cmp for that, that's only >> difference. >> >> This series is intended to unbind CPU cycle from time API, not change >> time API at once to be wall time. >> If we can guarantee time count cannot overflow, than we don't need >> odp_time_cmp at all, we can use diff for that. >> But currently, we cannot guarantee it. In this series odp_time_t opaque >> type is added only with one main intention: >> ask application to use odp_time_diff() and not compare times directly. > > > Yes, agree. Still any API change should be forward compatible to global vs. local time, and absolute time vs. range. odp_time_cmp() compares only ranges. The ranges have same type as timestamp, no need to add complexity here. odp_time_diff() takes care about overflows, I thought it's clear. It supposed that it's not wall time, all is talking about that. Who is created it? Maybe he can clarify? > > > >> >> In another case all this API is not needed at all, it can be simplified >> to simple functions: >> >> odp_time(); >> odp_time_ns(uint64 time_stamp); >> odp_time_res(); >> >> That's all API. If time cannot overflow, then why diff?, Do simply t2 - >> t1 and live easily. >> As time never overflows, than we always can compare t1 and t2 w/o >> problems, why not, t1 < t2, >> it's simple and fast, faster cannot be. Need ns, use above great API >> function odp_time_ns(). >> Need to add local time? add additional one API, why not? >> odp_time_local_ns()..... >> >> Why this stuff is needed at all? It was added because time counter can >> overflow. >> How your implementation is going to control that? When you read next >> time, how do you know that >> it's overflowed?, remember specifically previous state, run additional >> kernel to read it periodically? >> If no - then use current API or don't use it at all. Why do we need to >> overload simple functions, aim of >> which simply measure short periods of time? check time type in each of >> them, extract some init values, >> control overflow...etc. > > > For sure the HW counter can overflow (32 bit counters or counter not init to zero). Stop. If you will try to add 32 bit counter you will see that it's not possible. I wanted to show existent examples here, but seems that simple logic is enough: Try to answer only on 3 first questions: - why current API doesn't return 32bit value, only 64bit - why current API doesn't have SUM function, a + b can be easily overflow 32 bit. - why current API get as input 64bit value. ..... So any 32 bit counters > Implementation can easily detect one wrap around and that's typically enough Really? Please, explain. It cannot. Especially if this value is 32 bit. You never know when you are going to read time next time and you don't have interrupts. > A 32 bit counter wraps in 4 seconds at 1 GHz. That's one of the reasons why it cannot be used for such purposes. It could be used only if it's rate small enough to match overflow time more then several hours, at least > It can be an implementation configuration time trade off to select high resolution vs. long wrap around time (longest possible interval to measure). > Global and local time can have different config or limitations (documented in release notes). Once again. Here I'm talking only about Global timer. > > >> >>>> >>>>> >>>>> So, t2 >= t1 and function returns how much later t2 was stamped >> from >>>> t1. >>>> >>>> On my opinion it's correctly described in current version. No see >>>> reason to change it. >>>> * @param t1 First time stamp >>>> * @param t2 Second time stamp >>> >>> >>> It's half way there, but should be very clear that t1 is always >> stamped before t2 (or at the same time, never after t2). >> >> It's not correct to hardly write it here. >> It can be used to diff time ranges, not only timestamps. >> Probably better simply name it: >>>> * @param t1 First time >>>> * @param t2 Second time >> >> and add some line like: if diff timestamps (most cases) t2 is supposed >> to be got after t1. >> >>> >>> >>>> >>>>> >>>>>> >>>>>> /** >>>>>> * Time sum >>>>>> * >>>>>> * @param t1 First time stamp >>>>>> * @param t2 Second time stamp >>>>>> * >>>>>> * @return Sum of time stamps in time handles >>>>>> */ >>>>>> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2); >>>>> >>>>> Here order does not matter and it should be also documented. >>>>> >>>> >>>> Yep. I'll update. >>>> >>>>> >>>>>> >>>>>> /** >>>>>> * Convert time handle to nanoseconds >>>>>> * >>>>>> * @param time Time handle representing time >>>>>> * >>>>>> * @return Time in nanoseconds >>>>>> */ >>>>>> uint64_t odp_time_to_ns(odp_time_t time); >>>>> >>>>> >>>>> // convert time to nsec >>>>> // nsec value should start from 0 in ODP init (nsec value must not >>>> wrap in several years) >>>>> // Global and local time may result different nsec values >> (different >>>> time) >>>>> >>>>> Nsec time should not wrap in the near future (e.g. for next 58 >> years >>>> @ 10GHz == 64 bits). So the nsec time value should be reseted to 0 >> in >>>> some point in ODP initialization. This is needs to be documented. >>>> >>>> By default h/w counter is reseted at init. But we shouldn't >> guarantee >>>> it. >>>> Some SDK cannot allow to reset timer, only read, so >>>> if counter is assigned 0 at bootloader stage, it can be equal to ~ >> 0- >>>> 10sec ~ 0-100000000000ns, >>>> when your application's started (it can be started any time). You >> can >>>> have two applications >>>> that started at different time....etc. >>>> >>>> So you cannot add such strict requirement, as in another case it >>>> requires an implementation to remember >>>> some init value X, and every time when odp_time() is called extract >>>> this X from current timestamp. >>>> This is not acceptable in cases when this is used to measure small >>>> times in s/w cycles. In most cases >>>> we need only relative time, cases when we need wall time can be >> counted >>>> on the fingers. >>>> For log function, if it's needed, we can remember init value X and >>>> extract it each time when print, >>>> but it can be done only for this use-case and others use-cases >> (which >>>> most) can be free from this redundancy. >>> >>> odp_time_t is there to give freedom for implementation to use >> whatever counter, not require to reset it, etc. >>> Only the wall clock time in nsec (msec, sec, ...) must not wrap. We >> need to spec it so that only the conversion function would do the >> extraction. >>> >>> // 64 bit time counter value at start 0xdead0000 >>> // counter runs at 1GHz >>> odp_init_global(); >>> >>> // store counter value = 100 000 + 0xdead0000 >>> t1 = odp_time(); >>> >>> // store counter value = 101 000 + 0xdead0000 >>> t2 = odp_time(); >>> >>> // (100 000 + 0xdead0000) - 0xdead0000 >>> // return 100 000 nsec >> >> You meant return 100 000/rate; > > Yes, in this simple case counter runs at 1 GHz: 1 cycles = 1 nsec. > >> >>> nsec1 = odp_time_to_ns(t1); >>> >>> // (101 000 + 0xdead0000) - 0xdead0000 >>> // return 101 000 nsec >> >> Again, about overflows, it be a little harder in implementation. >> >> what if time value were overflowed? >> Say you current time_stamp X is < 200? >> >> X - 0xdead0000 = 0xbaddead0 >> >> Even if not less than 0xdead0000, it will extract, but in fact it will >> be incorrect. >> It should be rewritten like: >> return (odp_time_diff(0xdead0000 - odp_time()))/rate; > > > 1 - 0xdead0000 = 0x2154000 nsec, which is distance from 0xdead0000 to 1 at 1 GHz. > The simple math can handle one wrap around. It doesn't matter have much warps, matters only spent time to handle it. Even more, wrap can be only one. You cannot catch more than one wrap, you cannot guarantee it w/o interrupt. You should rely on interrupt, thus it's impossible. > It's up to the implementation how many wraps it supports and on which resolution. I think one wrap is OK, but needs to be clearly documented. What shouldn't be documented it's number of warps. I'm not going even think about that. > > On 64 bit systems, one wrap is all you need. The next wrap happens e.g. in 580 year... As I sad, doesn't matter how much wraps. One wrap odp_time_diff function solves w/o any problem: wait_time = odp_time_from_ns(1); start_time = odp_time(); do { time = odp_time(); diff = odp_time_diff(start_time, time); } while (odp_time_cmp(diff, wait_time) > 0) > >> >> That's why better to use this function to convert ranges, that is >> relative time. >> >> Say you need print time in log: >> remember in odp_init_global(); >> start_time = odp_time(); >> >> then in print function: >> >> print ("%ll bla-bla-bla", odp_time_ns(odp_time_diff(start_time - >> odp_time()))); >> and no need to worry about wall time and overlaps and additional >> actions in other places >> where it's not needed. > > Global time and wraps is clearly better to solve once and in the implementation, instead of everyone solving it separately. It's better but not possible, why I've explained already. Every one should do it separably, for better performance, if don't know how to do it - see examples. that the idea initially included in time API and I'm not going to change it. It's not so hard. > It's just matter of correct API definition to support all use cases with good performance. Good performance - is the main criteria that suffer from adding redundancy. > > >> >>> nsec2 = odp_time_to_ns(t1); >>> >>> // diff = (101 000 + 0xdead0000) - (100 000 + 0xdead0000) = 1000 >>> // store 1 000 + 0xdead0000 >> >> That's the moment I'm worrying about. >> Each time you need to do + in some cycle. >> Thanks that it's not in odp_time(), it makes me worry a little less. > > Since it's stored in to odp_time_t, you can also store 1000 and a flag telling that it's a range instead of an absolute value. No, I dislike it. I tend to use current variant and avoid redundant checks. > It's up to implementation. We could have separate set of function calls for ranges and absolute values, but does it pay off. I think the counter read is taking most of the cycles anyway, at least when it's not local to CPU. We shouldn't relay on how fast access to the counter. > The rate division may also take some cycles. That's why, where it's sensible, we simply convert ns to ticks before entering a loop. So, it's not applicable here. > Simple add/sub operations are fast when the (constant) data is already in cache. > >> >>> t3 = odp_time_diff(t1, t2); >>> >>> // (1 000 + 0xdead0000) - 0xdead0000 >>> // return 1 000 nsec >>> nsec3 = odp_time_to_ns(t3); >>> >>> >>> >>> >>>> >>>>> >>>>>> >>>>>> /** >>>>>> * Convert nanoseconds to time of best hi-resolution time >> source >>>>>> available >>>>>> * >>>>>> * @param ns Time in nanoseconds >>>>>> * >>>>>> * @return Time handle representing time >>>>>> */ >>>>>> 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 t1 < t2, 0 if t1 = t2, 1 if t1 > t2 >>>>>> */ >>>>>> int odp_time_cmp(odp_time_t t1, odp_time_t t2); >>>>> >>>>> // Compare time values >>>>> // -1: t2 < t1 >>>>> // 0: t2 == t1 >>>>> // 1: t2 > t1 >>>>> >>>>> Again, order matters (user tests the order here) and it's better to >>>> keep same order/spec throughout the API >>>> >>>> No objection. I will correct. >>>> Initially I've taken this from strcmp...it's in backward order >> there. >>> >>> Yes, looked also strcmp but this order is more logical here. >>> >>> >>>> >>>>> >>>>> if (cpm(t1, t2) >= 0) { >>>>> // normal case, t2 > t1 >>>>> // foo expects that t2 >= t1 >>>>> foo(t1, t2) >>>>> } else { >>>>> // not the normal case, t2 < t1 >>>>> foo(t2, t1) >>>>> } >>>>> >>>>> >>>>> When order matter "t1" is specified to happen first in time and is >>>> left from t2 (in function parameters). >>>>> >>>>> >>>>>> >>>>>> /** >>>>>> * Get printable value for an odp_time_t >>>>>> * >>>>>> * @param hdl odp_time_t handle to be printed >>>>>> * @return uint64_t value that can be used to print/display >>>> this >>>>>> * handle >>>>>> * >>>>>> * @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 handle. >>>>>> */ >>>>>> uint64_t odp_time_to_u64(odp_time_t hdl); >>>>> >>>>> Nsec is also printable, so not sure how much this is needed. Maybe >>>> for debugging conversion functions... >>>> >>>> It's widely used in odp tests/examples for debugg purposes. >>>> W/o this call I cannot port new API correctly. >>> >>> >>> In general, odp_time_t can be a struct larger than 64 bits. For >> example, >>> >>> typedef struct { >>> uint8_t flags; // global/local time, wrapped/not wrapped, etc ... >>> uint64_t hw_counter_value; >>> } >> >> Then here we just return hw_counter_value. User who use this function >> should >> know what he is doing. > > Just saying that these 64 bits are for debugging. If the counter value is already 64 bits, you need to leave out some data. The bug may be hiding in a corrupted flag bit that was not output ... As I sad I'm not going to add this flag, at least now. Event if this flag is present, user knows what kind of time he is debugging. This function is not for wide use, as explained in comment. > > -Petri >
diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c index d392925..3d34210 100644 --- a/example/ipsec/odp_ipsec.c +++ b/example/ipsec/odp_ipsec.c @@ -293,12 +293,12 @@ odp_event_t polled_odp_schedule(odp_queue_t *from, uint64_t wait) break; if (0 == start_cycle) { - start_cycle = odp_time_cycles(); + start_cycle = odp_time_tick(); continue; } - cycle = odp_time_cycles(); - diff = odp_time_diff_cycles(start_cycle, cycle); + cycle = odp_time_tick(); + diff = odp_time_ticks_diff(start_cycle, cycle); if (wait < diff) break; diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c index 5d362ec..c6c809d 100644 --- a/example/timer/odp_timer_test.c +++ b/example/timer/odp_timer_test.c @@ -440,19 +440,19 @@ int main(int argc, char *argv[]) printf("CPU freq %"PRIu64" Hz\n", odp_sys_cpu_hz()); printf("Cycles vs nanoseconds:\n"); ns = 0; - cycles = odp_time_ns_to_cycles(ns); + cycles = odp_time_ns_to_tick(ns); printf(" %12"PRIu64" ns -> %12"PRIu64" cycles\n", ns, cycles); printf(" %12"PRIu64" cycles -> %12"PRIu64" ns\n", cycles, - odp_time_cycles_to_ns(cycles)); + odp_time_tick_to_ns(cycles)); for (ns = 1; ns <= 100*ODP_TIME_SEC; ns *= 10) { - cycles = odp_time_ns_to_cycles(ns); + cycles = odp_time_ns_to_tick(ns); printf(" %12"PRIu64" ns -> %12"PRIu64" cycles\n", ns, cycles); printf(" %12"PRIu64" cycles -> %12"PRIu64" ns\n", cycles, - odp_time_cycles_to_ns(cycles)); + odp_time_tick_to_ns(cycles)); } printf("\n"); diff --git a/include/odp/api/time.h b/include/odp/api/time.h index b0072fc..b48dcae 100644 --- a/include/odp/api/time.h +++ b/include/odp/api/time.h @@ -30,11 +30,11 @@ extern "C" { /** - * Current time in CPU cycles + * Current time in ticks of best hi-resolution timer available * - * @return Current time in CPU cycles + * @return Current time in timer ticks */ -uint64_t odp_time_cycles(void); +uint64_t odp_time_tick(void); /** @@ -43,29 +43,29 @@ 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 in timer ticks */ -uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2); +uint64_t odp_time_ticks_diff(uint64_t t1, uint64_t t2); /** - * Convert CPU cycles to nanoseconds + * Convert timer ticks to nanoseconds * - * @param cycles Time in CPU cycles + * @param ticks Time in timer ticks * * @return Time in nanoseconds */ -uint64_t odp_time_cycles_to_ns(uint64_t cycles); +uint64_t odp_time_tick_to_ns(uint64_t ticks); /** - * Convert nanoseconds to CPU cycles + * Convert nanoseconds to ticks of best hi-resolution timer available * * @param ns Time in nanoseconds * - * @return Time in CPU cycles + * @return Time in timer ticks */ -uint64_t odp_time_ns_to_cycles(uint64_t ns); +uint64_t odp_time_ns_to_tick(uint64_t ns); /** * @} diff --git a/test/performance/odp_pktio_perf.c b/test/performance/odp_pktio_perf.c index b09055f..b3f8d2b 100644 --- a/test/performance/odp_pktio_perf.c +++ b/test/performance/odp_pktio_perf.c @@ -325,28 +325,28 @@ static void *run_thread_tx(void *arg) if (outq == ODP_QUEUE_INVALID) LOG_ABORT("Failed to get output queue for thread %d\n", thr_id); - burst_gap_cycles = odp_time_ns_to_cycles( + burst_gap_cycles = odp_time_ns_to_tick( ODP_TIME_SEC / (targs->pps / targs->batch_len)); odp_barrier_wait(&globals->tx_barrier); - cur_cycles = odp_time_cycles(); + cur_cycles = odp_time_tick(); next_tx_cycles = cur_cycles; end_cycles = cur_cycles + - odp_time_ns_to_cycles(targs->duration * ODP_TIME_SEC); + odp_time_ns_to_tick(targs->duration * ODP_TIME_SEC); while (cur_cycles < end_cycles) { unsigned alloc_cnt = 0, tx_cnt; if (cur_cycles < next_tx_cycles) { - cur_cycles = odp_time_cycles(); + cur_cycles = odp_time_tick(); if (idle_start == 0) idle_start = cur_cycles; continue; } if (idle_start) { - stats->s.idle_cycles += odp_time_diff_cycles( + stats->s.idle_cycles += odp_time_ticks_diff( idle_start, cur_cycles); idle_start = 0; } @@ -362,14 +362,14 @@ static void *run_thread_tx(void *arg) stats->s.enq_failures += unsent_pkts; stats->s.tx_cnt += tx_cnt; - cur_cycles = odp_time_cycles(); + cur_cycles = odp_time_tick(); } VPRINT(" %02d: TxPkts %-8"PRIu64" EnqFail %-6"PRIu64 " AllocFail %-6"PRIu64" Idle %"PRIu64"ms\n", thr_id, stats->s.tx_cnt, stats->s.enq_failures, stats->s.alloc_failures, - odp_time_cycles_to_ns(stats->s.idle_cycles)/1000/1000); + odp_time_tick_to_ns(stats->s.idle_cycles) / 1000 / 1000); return NULL; } @@ -587,8 +587,9 @@ static int setup_txrx_masks(odp_cpumask_t *thd_mask_tx, */ static void busy_loop_ns(uint64_t wait_ns) { - uint64_t end = odp_time_cycles() + odp_time_ns_to_cycles(wait_ns); - while (odp_time_cycles() < end) + uint64_t end = odp_time_tick() + odp_time_ns_to_tick(wait_ns); + + while (odp_time_tick() < end) ; } diff --git a/test/performance/odp_scheduling.c b/test/performance/odp_scheduling.c index c425024..e34cea9 100644 --- a/test/performance/odp_scheduling.c +++ b/test/performance/odp_scheduling.c @@ -185,7 +185,7 @@ static int test_alloc_single(int thr, odp_pool_t pool) odp_buffer_t temp_buf; uint64_t t1, t2, cycles, ns; - t1 = odp_time_cycles(); + t1 = odp_time_tick(); for (i = 0; i < ALLOC_ROUNDS; i++) { temp_buf = odp_buffer_alloc(pool); @@ -198,9 +198,9 @@ static int test_alloc_single(int thr, odp_pool_t pool) odp_buffer_free(temp_buf); } - t2 = odp_time_cycles(); - cycles = odp_time_diff_cycles(t1, t2); - ns = odp_time_cycles_to_ns(cycles); + t2 = odp_time_tick(); + cycles = odp_time_ticks_diff(t1, t2); + ns = odp_time_tick_to_ns(cycles); printf(" [%i] alloc_sng alloc+free %"PRIu64" cycles, %"PRIu64" ns\n", thr, cycles/ALLOC_ROUNDS, ns/ALLOC_ROUNDS); @@ -222,7 +222,7 @@ static int test_alloc_multi(int thr, odp_pool_t pool) odp_buffer_t temp_buf[MAX_ALLOCS]; uint64_t t1, t2, cycles, ns; - t1 = odp_time_cycles(); + t1 = odp_time_tick(); for (i = 0; i < ALLOC_ROUNDS; i++) { for (j = 0; j < MAX_ALLOCS; j++) { @@ -238,9 +238,9 @@ static int test_alloc_multi(int thr, odp_pool_t pool) odp_buffer_free(temp_buf[j-1]); } - t2 = odp_time_cycles(); - cycles = odp_time_diff_cycles(t1, t2); - ns = odp_time_cycles_to_ns(cycles); + t2 = odp_time_tick(); + cycles = odp_time_ticks_diff(t1, t2); + ns = odp_time_tick_to_ns(cycles); printf(" [%i] alloc_multi alloc+free %"PRIu64" cycles, %"PRIu64" ns\n", thr, cycles/(ALLOC_ROUNDS*MAX_ALLOCS), @@ -289,7 +289,7 @@ static int test_poll_queue(int thr, odp_pool_t msg_pool) return -1; } - t1 = odp_time_cycles(); + t1 = odp_time_tick(); for (i = 0; i < QUEUE_ROUNDS; i++) { ev = odp_buffer_to_event(buf); @@ -310,9 +310,9 @@ static int test_poll_queue(int thr, odp_pool_t msg_pool) } } - t2 = odp_time_cycles(); - cycles = odp_time_diff_cycles(t1, t2); - ns = odp_time_cycles_to_ns(cycles); + t2 = odp_time_tick(); + cycles = odp_time_ticks_diff(t1, t2); + ns = odp_time_tick_to_ns(cycles); printf(" [%i] poll_queue enq+deq %"PRIu64" cycles, %"PRIu64" ns\n", thr, cycles/QUEUE_ROUNDS, ns/QUEUE_ROUNDS); @@ -348,7 +348,7 @@ static int test_schedule_single(const char *str, int thr, if (create_queue(thr, msg_pool, prio)) return -1; - t1 = odp_time_cycles(); + t1 = odp_time_tick(); for (i = 0; i < QUEUE_ROUNDS; i++) { ev = odp_schedule(&queue, ODP_SCHED_WAIT); @@ -382,9 +382,9 @@ static int test_schedule_single(const char *str, int thr, odp_schedule_resume(); - t2 = odp_time_cycles(); - cycles = odp_time_diff_cycles(t1, t2); - ns = odp_time_cycles_to_ns(cycles); + t2 = odp_time_tick(); + cycles = odp_time_ticks_diff(t1, t2); + ns = odp_time_tick_to_ns(cycles); odp_barrier_wait(barrier); clear_sched_queues(); @@ -429,7 +429,7 @@ static int test_schedule_many(const char *str, int thr, return -1; /* Start sched-enq loop */ - t1 = odp_time_cycles(); + t1 = odp_time_tick(); for (i = 0; i < QUEUE_ROUNDS; i++) { ev = odp_schedule(&queue, ODP_SCHED_WAIT); @@ -463,9 +463,9 @@ static int test_schedule_many(const char *str, int thr, odp_schedule_resume(); - t2 = odp_time_cycles(); - cycles = odp_time_diff_cycles(t1, t2); - ns = odp_time_cycles_to_ns(cycles); + t2 = odp_time_tick(); + cycles = odp_time_ticks_diff(t1, t2); + ns = odp_time_tick_to_ns(cycles); odp_barrier_wait(barrier); clear_sched_queues(); @@ -547,7 +547,7 @@ static int test_schedule_multi(const char *str, int thr, } /* Start sched-enq loop */ - t1 = odp_time_cycles(); + t1 = odp_time_tick(); for (i = 0; i < QUEUE_ROUNDS; i++) { num = odp_schedule_multi(&queue, ODP_SCHED_WAIT, ev, @@ -584,9 +584,9 @@ static int test_schedule_multi(const char *str, int thr, odp_schedule_resume(); - t2 = odp_time_cycles(); - cycles = odp_time_diff_cycles(t1, t2); - ns = odp_time_cycles_to_ns(cycles); + t2 = odp_time_tick(); + cycles = odp_time_ticks_diff(t1, t2); + ns = odp_time_tick_to_ns(cycles); odp_barrier_wait(barrier); clear_sched_queues(); @@ -738,7 +738,7 @@ static void test_time(void) } while (tp1.tv_sec == tp2.tv_sec); - t1 = odp_time_cycles(); + t1 = odp_time_tick(); do { if (clock_gettime(CLOCK_MONOTONIC, &tp2)) { @@ -748,7 +748,7 @@ static void test_time(void) } while ((tp2.tv_sec - tp1.tv_sec) < TEST_SEC); - t2 = odp_time_cycles(); + t2 = odp_time_tick(); ns1 = (tp2.tv_sec - tp1.tv_sec)*1000000000; @@ -757,14 +757,14 @@ static void test_time(void) else ns1 -= tp1.tv_nsec - tp2.tv_nsec; - cycles = odp_time_diff_cycles(t1, t2); - ns2 = odp_time_cycles_to_ns(cycles); + cycles = odp_time_ticks_diff(t1, t2); + ns2 = odp_time_tick_to_ns(cycles); err = ((double)(ns2) - (double)ns1) / (double)ns1; printf("clock_gettime %"PRIu64" ns\n", ns1); - printf("odp_time_cycles %"PRIu64" cycles\n", cycles); - printf("odp_time_cycles_to_ns %"PRIu64" ns\n", ns2); + printf("odp_time_tick %"PRIu64" cycles\n", cycles); + printf("odp_time_tick_to_ns %"PRIu64" ns\n", ns2); printf("odp get cycle error %f%%\n", err*100.0); printf("\n"); diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index dc8d427..e32c7df 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -305,15 +305,15 @@ static odp_event_t queue_deq_wait_time(odp_queue_t queue, uint64_t ns) uint64_t start, now, diff; odp_event_t ev; - start = odp_time_cycles(); + start = odp_time_tick(); do { ev = odp_queue_deq(queue); if (ev != ODP_EVENT_INVALID) return ev; - now = odp_time_cycles(); - diff = odp_time_diff_cycles(start, now); - } while (odp_time_cycles_to_ns(diff) < ns); + now = odp_time_tick(); + diff = odp_time_ticks_diff(start, now); + } while (odp_time_tick_to_ns(diff) < ns); return ODP_EVENT_INVALID; } @@ -325,7 +325,7 @@ static odp_packet_t wait_for_packet(odp_queue_t queue, odp_event_t ev; odp_packet_t pkt = ODP_PACKET_INVALID; - start = odp_time_cycles(); + start = odp_time_tick(); do { if (queue != ODP_QUEUE_INVALID && @@ -345,9 +345,9 @@ static odp_packet_t wait_for_packet(odp_queue_t queue, odp_event_free(ev); } - now = odp_time_cycles(); - diff = odp_time_diff_cycles(start, now); - } while (odp_time_cycles_to_ns(diff) < ns); + now = odp_time_tick(); + diff = odp_time_ticks_diff(start, now); + } while (odp_time_tick_to_ns(diff) < ns); CU_FAIL("failed to receive transmitted packet"); diff --git a/test/validation/scheduler/scheduler.c b/test/validation/scheduler/scheduler.c index fe03ab7..5fb711f 100644 --- a/test/validation/scheduler/scheduler.c +++ b/test/validation/scheduler/scheduler.c @@ -215,7 +215,7 @@ static void *schedule_common_(void *arg) uint64_t cycles = 0; /* Do some work here to keep the thread busy */ for (cnt = 0; cnt < 1000; cnt++) - cycles += odp_time_cycles(); + cycles += odp_time_tick(); odp_spinlock_unlock(&globals->atomic_lock); } diff --git a/test/validation/time/time.c b/test/validation/time/time.c index 0aac599..a6169aa 100644 --- a/test/validation/time/time.c +++ b/test/validation/time/time.c @@ -18,16 +18,16 @@ static void time_test_odp_cycles_diff(void) volatile int count = 0; uint64_t diff, cycles1, cycles2; - cycles1 = odp_time_cycles(); + cycles1 = odp_time_tick(); while (count < BUSY_LOOP_CNT) { count++; }; - cycles2 = odp_time_cycles(); + cycles2 = odp_time_tick(); CU_ASSERT(cycles2 > cycles1); - diff = odp_time_diff_cycles(cycles1, cycles2); + diff = odp_time_ticks_diff(cycles1, cycles2); CU_ASSERT(diff > 0); } @@ -38,7 +38,7 @@ static void time_test_odp_cycles_negative_diff(void) cycles1 = 10; cycles2 = 5; - diff = odp_time_diff_cycles(cycles1, cycles2); + diff = odp_time_ticks_diff(cycles1, cycles2); CU_ASSERT(diff > 0); } @@ -49,10 +49,10 @@ static void time_test_odp_time_conversion(void) uint64_t upper_limit, lower_limit; ns1 = 100; - cycles = odp_time_ns_to_cycles(ns1); + cycles = odp_time_ns_to_tick(ns1); CU_ASSERT(cycles > 0); - ns2 = odp_time_cycles_to_ns(cycles); + ns2 = odp_time_tick_to_ns(cycles); /* need to check within arithmetic tolerance that the same * value in ns is returned after conversions */
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 frequency can be different from CPU frequency. 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 use with CPU. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- example/ipsec/odp_ipsec.c | 6 ++-- example/timer/odp_timer_test.c | 8 ++--- include/odp/api/time.h | 22 ++++++------- test/performance/odp_pktio_perf.c | 19 +++++------ test/performance/odp_scheduling.c | 60 +++++++++++++++++------------------ test/validation/pktio/pktio.c | 16 +++++----- test/validation/scheduler/scheduler.c | 2 +- test/validation/time/time.c | 12 +++---- 8 files changed, 73 insertions(+), 72 deletions(-)