And more: --8<---------------cut here---------------start------------->8--- @@ -323,7 +323,7 @@ step_systime( struct timespec ofs_ts; /* desired offset as teimspec */ /* get the complete jump distance as timespec */ - ofs_ts = d_to_tspec((step + sys_residual + 0.5e-9) * 1e9); + ofs_ts = d_to_tspec(step + sys_residual + 0.5e-9); /* ---> time-critical path starts ---> */ --8<---------------cut here---------------end--------------->8---
Given how you've defined d_to_tspec, adding half a nanosecond is another error right there since it's done again in d_to_tspec. You are no longer rounding, but instead addding a whole nanosecond. Please fix it up in another commit. Oh, and you really do not need to normalize in d_to_tspec if you take care of the fact that the nanoseconds should always be positive. That's true by definition if you start with a positive number and should be handled directly in the conversion otherwise. The normalization function itself has a pretty big blooper as well that has been commited by Eric (dropping a minus sign in the condition). It basically made the test fail every time (except when the value was eaxactly NANOSECOND), so we've run through the normalization even when the number was already normalized. If micro optimizations aren't interesting, we should at least implement it correctly. Since C99 has a function that should be highly optimized that provides the numbers we need, use it. So, how about this (the code for 32bit timespec might be removed later and replaced with a variant using div instead of LDIV):
>From c0099adc2fb1e98162ae4dc1a274a6a5afc7c480 Mon Sep 17 00:00:00 2001 From: Achim Gratz <strom...@stromeko.de> Date: Sat, 11 Mar 2017 13:10:08 +0100 Subject: [PATCH 1/2] Fix normalize_tspec * include/timespecops.h (normalize_tspec): Commit d2d2b531 disarmed the initial test by dropping a minus sign. Re-implement the normalization using ldiv library function that should be highly optimized already, especially on modern processors where this might be a single instruction. Unfortunately there is no variant that does Euclidean division, so we have to fix up the result for negative remainders ourselves. --- include/timespecops.h | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/include/timespecops.h b/include/timespecops.h index 03b8be3..76ebd26 100644 --- a/include/timespecops.h +++ b/include/timespecops.h @@ -77,25 +77,23 @@ normalize_tspec( ) { #if NTP_SIZEOF_LONG > 4 - long z; - /* * tv_nsec is of type 'long', and on a 64-bit machine using only * loops becomes prohibitive once the upper 32 bits get * involved. On the other hand, division by constant should be * fast enough; so we do a division of the nanoseconds in that - * case. The floor adjustment step follows with the standard - * normalisation loops. And labs() is intentionally not used - * here: it has implementation-defined behaviour when applied - * to LONG_MIN. + * case. */ - if (x.tv_nsec < NANOSECONDS || - x.tv_nsec > NANOSECONDS) { - z = x.tv_nsec / NANOSECONDS; - x.tv_nsec -= z * NANOSECONDS; - x.tv_sec += z; + if (x.tv_nsec < 0 || x.tv_nsec >= NANOSECONDS) { + ldiv_t z = ldiv( x.tv_nsec, NANOSECONDS); + if (z.rem < 0) { + z.quot--; + z.rem += NANOSECONDS; + } + x.tv_sec += z.quot; + x.tv_nsec = z.rem; } -#endif +#else /* since 10**9 is close to 2**32, we don't divide but do a * normalisation in a loop; this takes 3 steps max, and should * outperform a division even if the mul-by-inverse trick is @@ -110,6 +108,7 @@ normalize_tspec( x.tv_nsec -= NANOSECONDS; x.tv_sec++; } while (x.tv_nsec >= NANOSECONDS); +#endif return x; } -- 2.1.4
>From 6a00bc104c454016d8214d4cf36689b3b7777c5c Mon Sep 17 00:00:00 2001 From: Achim Gratz <strom...@stromeko.de> Date: Sat, 11 Mar 2017 13:11:18 +0100 Subject: [PATCH 2/2] Use floor in d_to_tspec * include/timespecops.h (d_to_tspec): Use standard function floor in d_to_tspec and also use NANOSECONDS instead of a magical constant. Normalization is no longer necessary as the result is normalized by construction, even for negative inputs. --- include/timespecops.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/include/timespecops.h b/include/timespecops.h index 76ebd26..6913feb 100644 --- a/include/timespecops.h +++ b/include/timespecops.h @@ -120,13 +120,11 @@ d_to_tspec( ) { struct timespec x; + double s = floor(d); - d += 0.5e-9; /* round on LSB */ - x.tv_sec = (time_t) d; - x.tv_nsec = (long)((d - (double)x.tv_sec) * 1e9); - - /* unlikely, but ensure it is normalized */ - return normalize_tspec(x); + x.tv_sec = (time_t) s; + x.tv_nsec = (long) (((d - s) * NANOSECONDS) + 0.5); + return x; } /* x = a + b */ -- 2.1.4
Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptation for Waldorf rackAttack V1.04R1: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
_______________________________________________ devel mailing list devel@ntpsec.org http://lists.ntpsec.org/mailman/listinfo/devel