On 04/26 08:02:09, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>
>
> > > > > diff --git a/platform/linux-
> > generic/include/odp/api/plat/time_types.h
> > > > b/platform/linux-generic/include/odp/api/plat/time_types.h
> > > > > index 4847f3b1..8ae7ce82 100644
> > > > > --- a/platform/linux-generic/include/odp/api/plat/time_types.h
> > > > > +++ b/platform/linux-generic/include/odp/api/plat/time_types.h
> > > > > @@ -26,11 +26,28 @@ extern "C" {
> > > > > * the linux timespec structure, which is dependent on POSIX
> > extension
> > > > level.
> > > > > */
> > > > > typedef struct odp_time_t {
> > > > > - int64_t tv_sec; /**< @internal Seconds */
> > > > > - int64_t tv_nsec; /**< @internal Nanoseconds */
> > > > > + union {
> > > > > + /** @internal Posix timespec */
> > > > > + struct {
> > > > > + /** @internal Seconds */
> > > > > + int64_t tv_sec;
> > > > > +
> > > > > + /** @internal Nanoseconds */
> > > > > + int64_t tv_nsec;
> > > > > + } spec;
> > > > > +
> > > > > + /** @internal HW time counter */
> > > > > + struct {
> > > > > + /** @internal Counter value */
> > > > > + uint64_t count;
> > > > > +
> > > > > + /** @internal Reserved */
> > > > > + uint64_t res;
> > > > > + } hw;
> > > > > + };
> > > >
> > > > A processor's tick counter cannot be used to measure calendar time by
> > > > itself. The delta between two ticks, however, can be converted to
> > > > calendar time.
> > > >
> > > > Please see the proposal that introduces odp_tick_t which eliminates
> > > > the wasted u64 reserved field in the union above.
> > >
> > >
> > > Nothing is wasted here today.
> >
> > The 64-bit CPU time is stored in a 128-bit data structure where 64-bits
> > are wasted. You can use just a 64-bit type and then convert that to
> > a timespec (using a starting timestamp taken on global init) if needed.
> >
>
> Nothing is wasted compared to the situation today. Entire timespec is stored
> as before. If you want to compress timespec, it's another topic. Compress
> means additional complexity and overhead. This patch set just adds ability to
> use 64 but HW time when available. Timespec side implementation remains as is.
You are re-using the calendar time type to store a value read from the CPU
counter. A different approach is to use a different type (64 bits only for no
wasted space) for the value read from the CPU.
This value must be converted to a calendar time time if needed. It cannot just
be used to represent calendar time.
However, for some applications and use cases, you don't need to convert to
calendar time
in order to measure across two reads of the counter. You can also do direct
arithmetic
on the value instead of arithmetic on a timespec.
If you want to read code instead of email or design documentation, look at this:
https://git.linaro.org/people/ola.liljedahl/sw_scheduler.git/tree/time_bench.c
https://git.linaro.org/people/ola.liljedahl/sw_scheduler.git/tree/time.h
All you need is a 64 bit type, a way to read from the counter as well as get
the frequency, and 2 very small conversion functions.
> > > This is because all our code (in this and other files) does "static
> > inline" for functions that we want to be inlined. Static is used for
> > functions that are static, but we don't care if those are inlined (slow
> > path). Most time API functions are fast path.
> >
> > I am saying that adding the 'inline' qualifier here doesn't actually do
> > anything
> > because the compiler will consider static functions for inlining anyways.
>
>
> We all know that those are considered. We use "inline" as C standard
> describes: to suggest which function calls should be as fast as possible.
>
>
> > > > > +
> > > > > +static inline odp_time_t time_hw_sum(odp_time_t t1, odp_time_t t2)
> > > > > +{
> > > > > + odp_time_t time;
> > > > > +
> > > > > + time.hw.res = 0;
> > > > > + time.hw.count = t1.hw.count + t2.hw.count;
> > > > > +
> > > > > + return time;
> > > > > +}
> > > >
> > > > If a single u64 were used this code wouldn't need to exist.
> > >
> > > Which code? hw.res = 0 ? It not actually used for anything and could be
> > removed. Although, the performance gain would not be huge. Anyway, I'll
> > remove it for v2.
> >
> > If the CPU time was stored in a 64-bit type, you can use arithmetic
> > operators
> > on the values directly instead of doing arithmetic on a 128-bit compound
> > datatype where 64-bits are unused. This is the union above.
>
>
> The implementation above does 64bit arithmetic. The reserved field is not
> used for anything. V2 removed all .res references (except one due to a false
> GCC warning). We have abstract API definition, so that application remains
> portable also, when time cannot be represented with a single integer type.
>
> It's implementation defined, if e.g. the above sum function is inlined and
> thus results no overhead at all.
>
> -Petri
>
>
