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

Reply via email to