fix(timestamp): eliminate f64 timebase drift and mantissa truncation in long recordings (#2144)

crates/timestamp/src/macos.rs

@@ -1,55 +1,68 @@
 use cidre::mach::TimeBaseInfo;
 use std::{
     ops::{Add, Sub},
+    sync::OnceLock,
     time::Duration,
 };
 
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub struct MachAbsoluteTimestamp(
-    // Nanoseconds
+    // Raw mach_absolute_time ticks
     u64,
 );
 
+static TIMEBASE_RATIO: OnceLock<(u128, u128)> = OnceLock::new();
+
+#[inline]
+fn timebase_ratio() -> (u128, u128) {
+    *TIMEBASE_RATIO.get_or_init(|| {
+        let info = TimeBaseInfo::new();
+        (u128::from(info.numer.max(1)), u128::from(info.denom.max(1)))
+    })
+}
+
+#[inline]
+fn ticks_to_nanos_with_ratio(ticks: u64, numer: u128, denom: u128) -> u64 {
+    let scaled = u128::from(ticks) * numer + (denom / 2);
+    (scaled / denom) as u64
+}
+
+#[inline]
+fn nanos_to_ticks_with_ratio(nanos: u128, numer: u128, denom: u128) -> u64 {
+    let scaled = nanos * denom + (numer / 2);
+    (scaled / numer) as u64
+}
+
 impl MachAbsoluteTimestamp {
-    pub fn new(nanos: u64) -> Self {
-        Self(nanos)
+    pub fn new(ticks: u64) -> Self {
+        Self(ticks)
     }
 
     pub fn now() -> Self {
         Self(cidre::mach::abs_time())
     }
 
     pub fn duration_since(&self, other: Self) -> Duration {
-        let info = TimeBaseInfo::new();
-        let freq = info.numer as f64 / info.denom as f64;
-
-        let Some(diff) = self.0.checked_sub(other.0) else {
-            return Duration::ZERO;
-        };
-
-        Duration::from_nanos((diff as f64 * freq) as u64)
+        self.checked_duration_since(other).unwrap_or(Duration::ZERO)
     }
 
     pub fn checked_duration_since(&self, other: Self) -> Option<Duration> {
-        let info = TimeBaseInfo::new();
-        let freq = info.numer as f64 / info.denom as f64;
-
         let diff = self.0.checked_sub(other.0)?;
-
-        Some(Duration::from_nanos((diff as f64 * freq) as u64))
+        let (numer, denom) = timebase_ratio();
+        Some(Duration::from_nanos(ticks_to_nanos_with_ratio(
+            diff, numer, denom,
+        )))
     }
 
     pub fn signed_duration_since_secs(&self, other: Self) -> f64 {
-        let info = TimeBaseInfo::new();
-        let freq = info.numer as f64 / info.denom as f64;
-
-        let nanos = if self.0 >= other.0 {
-            ((self.0 - other.0) as f64 * freq) as i64
+        let (numer, denom) = timebase_ratio();
+        if self.0 >= other.0 {
+            let nanos = ticks_to_nanos_with_ratio(self.0 - other.0, numer, denom);
+            nanos as f64 / 1_000_000_000.0
         } else {
-            -(((other.0 - self.0) as f64 * freq) as i64)
-        };
-
-        nanos as f64 / 1_000_000_000.0
+            let nanos = ticks_to_nanos_with_ratio(other.0 - self.0, numer, denom);
+            -(nanos as f64 / 1_000_000_000.0)
+        }
     }
 
     pub fn from_cpal(instant: cpal::StreamInstant) -> Self {
@@ -63,24 +76,19 @@ impl Add<Duration> for MachAbsoluteTimestamp {
     type Output = Self;
 
     fn add(self, rhs: Duration) -> Self::Output {
-        let info = TimeBaseInfo::new();
-        // ns = ticks * numer / denom, so ticks = ns * denom / numer.
-        // On Apple Silicon (numer/denom = 125/3) multiplying by numer/denom
-        // instead would add ~1736x the intended duration.
-        let ticks = rhs.as_nanos() as f64 * info.denom as f64 / info.numer as f64;
-
-        Self((self.0 as f64 + ticks) as u64)
+        let (numer, denom) = timebase_ratio();
+        let delta_ticks = nanos_to_ticks_with_ratio(rhs.as_nanos(), numer, denom);
+        Self(self.0.saturating_add(delta_ticks))
     }
 }
 
 impl Sub<Duration> for MachAbsoluteTimestamp {
     type Output = Self;
 
     fn sub(self, rhs: Duration) -> Self::Output {
-        let info = TimeBaseInfo::new();
-        let ticks = rhs.as_nanos() as f64 * info.denom as f64 / info.numer as f64;
-
-        Self((self.0 as f64 - ticks).max(0.0) as u64)
+        let (numer, denom) = timebase_ratio();
+        let delta_ticks = nanos_to_ticks_with_ratio(rhs.as_nanos(), numer, denom);
+        Self(self.0.saturating_sub(delta_ticks))
     }
 }
 
@@ -128,4 +136,24 @@ mod tests {
             "add/sub of the same duration must cancel out, drifted {drift:?}"
         );
     }
+
+    #[test]
+    fn apple_silicon_timebase_preserves_precision_over_long_recordings() {
+        let numer = 125u128;
+        let denom = 3u128;
+        let target_ns: u128 = 3_600_000_123_456;
+        let step_ticks = nanos_to_ticks_with_ratio(target_ns, numer, denom);
+        let high_uptime = MachAbsoluteTimestamp::new((1u64 << 53) + 1);
+        assert_eq!(high_uptime + Duration::ZERO, high_uptime);
+        assert_eq!(high_uptime - Duration::ZERO, high_uptime);
+
+        let later = MachAbsoluteTimestamp::new(high_uptime.0 + step_ticks);
+        let elapsed_ns = ticks_to_nanos_with_ratio(later.0 - high_uptime.0, numer, denom);
+        assert!((elapsed_ns as i128 - target_ns as i128).abs() <= 42);
+
+        let delta = Duration::from_nanos(3_600_000_123_456);
+        let roundtrip = (high_uptime + delta).duration_since(high_uptime);
+        let err_ns = (roundtrip.as_nanos() as i128 - delta.as_nanos() as i128).abs();
+        assert!(err_ns <= 100, "unexpected operator drift: {err_ns}ns");
+    }
 }

crates/timestamp/src/win.rs

@@ -22,6 +22,12 @@ fn perf_freq() -> i64 {
     })
 }
 
+#[inline]
+fn duration_to_counter_ticks(duration: Duration, frequency: i64) -> i128 {
+    let freq = i128::from(frequency);
+    (duration.as_nanos() as i128 * freq + 500_000_000) / 1_000_000_000
+}
+
 impl PerformanceCounterTimestamp {
     pub fn new(value: i64) -> Self {
         Self(i128::from(value))
@@ -32,22 +38,7 @@ impl PerformanceCounterTimestamp {
     }
 
     pub fn duration_since(&self, other: Self) -> Duration {
-        let freq = perf_freq() as i128;
-        debug_assert!(freq > 0);
-
-        let diff = self.0 - other.0;
-
-        if diff <= 0 {
-            Duration::ZERO
-        } else {
-            let diff = diff as u128;
-            let freq = freq as u128;
-
-            let secs = diff / freq;
-            let nanos = ((diff % freq) * 1_000_000_000u128) / freq;
-
-            Duration::new(secs as u64, nanos as u32)
-        }
+        self.checked_duration_since(other).unwrap_or(Duration::ZERO)
     }
 
     pub fn checked_duration_since(&self, other: Self) -> Option<Duration> {
@@ -95,17 +86,15 @@ impl Add<Duration> for PerformanceCounterTimestamp {
     type Output = Self;
 
     fn add(self, rhs: Duration) -> Self::Output {
-        let freq = perf_freq();
-        Self(self.0 + (rhs.as_secs_f64() * freq as f64) as i128)
+        Self(self.0 + duration_to_counter_ticks(rhs, perf_freq()))
     }
 }
 
 impl Sub<Duration> for PerformanceCounterTimestamp {
     type Output = Self;
 
     fn sub(self, rhs: Duration) -> Self::Output {
-        let freq = perf_freq();
-        Self(self.0 - (rhs.as_secs_f64() * freq as f64) as i128)
+        Self(self.0 - duration_to_counter_ticks(rhs, perf_freq()))
     }
 }
 
@@ -165,4 +154,23 @@ mod tests {
             Duration::from_millis(10)
         );
     }
+
+    #[test]
+    fn duration_to_counter_ticks_preserves_long_recording_precision() {
+        let freq = 24_000_000i64;
+        let non_power_of_two = Duration::from_nanos(3_600_000_123_456);
+        let expected = (non_power_of_two.as_nanos() * u128::try_from(freq).unwrap() + 500_000_000)
+            / 1_000_000_000;
+        assert_eq!(
+            duration_to_counter_ticks(non_power_of_two, freq),
+            expected as i128
+        );
+
+        let high_base = PerformanceCounterTimestamp::new((1i64 << 53) + 1);
+        assert_eq!((high_base + Duration::ZERO).0, high_base.0);
+        assert_eq!((high_base - Duration::ZERO).0, high_base.0);
+        let roundtrip = (high_base + non_power_of_two).duration_since(high_base);
+        let err_ns = (roundtrip.as_nanos() as i128 - non_power_of_two.as_nanos() as i128).abs();
+        assert!(err_ns <= 1_000, "unexpected operator drift: {err_ns}ns");
+    }
 }