make Instant::{duration_since, elapsed, sub} saturating and remove workarounds

This removes all mutex/atomics based workarounds for non-monotonic clocks and makes the previously panicking methods saturating instead.

Effectively this moves the monotonization from `Instant` construction to the comparisons.

This has some observable effects, especially on platforms without monotonic clocks:

* Incorrectly ordered Instant comparisons no longer panic. This may hide some programming errors until someone actually looks at the resulting `Duration`
* `checked_duration_since` will now return `None` in more cases. Previously it only happened when one compared instants obtained in the wrong order or
  manually created ones. Now it also does on backslides.

The upside is reduced complexity and lower overhead of `Instant::now`.
This commit is contained in:
The8472 2021-10-15 23:55:23 +02:00 committed by The 8472
parent 5d8767cb22
commit 9d8ef11607
10 changed files with 30 additions and 270 deletions

View file

@ -115,14 +115,6 @@ pub fn now() -> Instant {
Instant { t: time }
}
pub const fn zero() -> Instant {
Instant { t: Timespec::zero() }
}
pub fn actually_monotonic() -> bool {
true
}
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
self.t.sub_timespec(&other.t).ok()
}

View file

@ -14,15 +14,6 @@ pub fn now() -> Instant {
}
}
pub const fn zero() -> Instant {
Instant(0)
}
pub fn actually_monotonic() -> bool {
// There are ways to change the system time
false
}
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
self.0.checked_sub(other.0).map(|ticks| {
// `SYSTIM` is measured in microseconds

View file

@ -25,14 +25,6 @@ pub fn checked_add_duration(&self, other: &Duration) -> Option<Instant> {
pub fn checked_sub_duration(&self, other: &Duration) -> Option<Instant> {
Some(Instant(self.0.checked_sub(*other)?))
}
pub fn actually_monotonic() -> bool {
false
}
pub const fn zero() -> Instant {
Instant(Duration::from_secs(0))
}
}
impl SystemTime {

View file

@ -154,14 +154,6 @@ pub fn now() -> Instant {
Instant { t: unsafe { mach_absolute_time() } }
}
pub const fn zero() -> Instant {
Instant { t: 0 }
}
pub fn actually_monotonic() -> bool {
true
}
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
let diff = self.t.checked_sub(other.t)?;
let info = info();
@ -296,17 +288,6 @@ pub fn now() -> Instant {
Instant { t: now(libc::CLOCK_MONOTONIC) }
}
pub const fn zero() -> Instant {
Instant { t: Timespec::zero() }
}
pub fn actually_monotonic() -> bool {
(cfg!(target_os = "linux") && cfg!(target_arch = "x86_64"))
|| (cfg!(target_os = "linux") && cfg!(target_arch = "x86"))
|| (cfg!(target_os = "linux") && cfg!(target_arch = "aarch64"))
|| cfg!(target_os = "fuchsia")
}
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
self.t.sub_timespec(&other.t).ok()
}

View file

@ -13,14 +13,6 @@ pub fn now() -> Instant {
panic!("time not implemented on this platform")
}
pub const fn zero() -> Instant {
Instant(Duration::from_secs(0))
}
pub fn actually_monotonic() -> bool {
false
}
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
self.0.checked_sub(other.0)
}

View file

@ -25,14 +25,6 @@ pub fn now() -> Instant {
Instant(current_time(wasi::CLOCKID_MONOTONIC))
}
pub const fn zero() -> Instant {
Instant(Duration::from_secs(0))
}
pub fn actually_monotonic() -> bool {
true
}
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
self.0.checked_sub(other.0)
}

View file

@ -41,14 +41,6 @@ pub fn now() -> Instant {
perf_counter::PerformanceCounterInstant::now().into()
}
pub fn actually_monotonic() -> bool {
false
}
pub const fn zero() -> Instant {
Instant { t: Duration::from_secs(0) }
}
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
// On windows there's a threshold below which we consider two timestamps
// equivalent due to measurement error. For more details + doc link,

View file

@ -31,7 +31,6 @@
#![stable(feature = "time", since = "1.3.0")]
mod monotonic;
#[cfg(test)]
mod tests;
@ -125,6 +124,23 @@
/// > structure cannot represent the new point in time.
///
/// [`add`]: Instant::add
///
/// ## Monotonicity
///
/// On all platforms `Instant` will try to use an OS API that guarantees monotonic behavior
/// if available, which is the case for all [tier 1] platforms.
/// In practice such guarantees are under rare circumstances broken by hardware, virtualization
/// or operating system bugs. To work around these bugs and platforms not offering monotonic clocks
/// [`duration_since`], [`elapsed`] and [`sub`] saturate to zero. [`checked_duration_since`] can
/// be used to detect and handle situations where monotonicity is violated, or `Instant`s are
/// subtracted in the wrong order.
///
/// [tier 1]: https://doc.rust-lang.org/rustc/platform-support.html
/// [`duration_since`]: Instant::duration_since
/// [`elapsed`]: Instant::elapsed
/// [`sub`]: Instant::sub
/// [`checked_duration_since`]: Instant::checked_duration_since
///
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[stable(feature = "time2", since = "1.8.0")]
pub struct Instant(time::Instant);
@ -247,59 +263,12 @@ impl Instant {
#[must_use]
#[stable(feature = "time2", since = "1.8.0")]
pub fn now() -> Instant {
let os_now = time::Instant::now();
// And here we come upon a sad state of affairs. The whole point of
// `Instant` is that it's monotonically increasing. We've found in the
// wild, however, that it's not actually monotonically increasing for
// one reason or another. These appear to be OS and hardware level bugs,
// and there's not really a whole lot we can do about them. Here's a
// taste of what we've found:
//
// * #48514 - OpenBSD, x86_64
// * #49281 - linux arm64 and s390x
// * #51648 - windows, x86
// * #56560 - windows, x86_64, AWS
// * #56612 - windows, x86, vm (?)
// * #56940 - linux, arm64
// * https://bugzilla.mozilla.org/show_bug.cgi?id=1487778 - a similar
// Firefox bug
//
// It seems that this just happens a lot in the wild.
// We're seeing panics across various platforms where consecutive calls
// to `Instant::now`, such as via the `elapsed` function, are panicking
// as they're going backwards. Placed here is a last-ditch effort to try
// to fix things up. We keep a global "latest now" instance which is
// returned instead of what the OS says if the OS goes backwards.
//
// To hopefully mitigate the impact of this, a few platforms are
// excluded as "these at least haven't gone backwards yet".
//
// While issues have been seen on arm64 platforms the Arm architecture
// requires that the counter monotonically increases and that it must
// provide a uniform view of system time (e.g. it must not be possible
// for a core to receive a message from another core with a time stamp
// and observe time going backwards (ARM DDI 0487G.b D11.1.2). While
// there have been a few 64bit SoCs that have bugs which cause time to
// not monoticially increase, these have been fixed in the Linux kernel
// and we shouldn't penalize all Arm SoCs for those who refuse to
// update their kernels:
// SUN50I_ERRATUM_UNKNOWN1 - Allwinner A64 / Pine A64 - fixed in 5.1
// FSL_ERRATUM_A008585 - Freescale LS2080A/LS1043A - fixed in 4.10
// HISILICON_ERRATUM_161010101 - Hisilicon 1610 - fixed in 4.11
// ARM64_ERRATUM_858921 - Cortex A73 - fixed in 4.12
if time::Instant::actually_monotonic() {
return Instant(os_now);
}
Instant(monotonic::monotonize(os_now))
Instant(time::Instant::now())
}
/// Returns the amount of time elapsed from another instant to this one.
/// Returns the amount of time elapsed from another instant to this one,
/// or zero duration if that instant is later than this one.
///
/// # Panics
///
/// This function will panic if `earlier` is later than `self`.
///
/// # Examples
///
@ -311,16 +280,22 @@ pub fn now() -> Instant {
/// sleep(Duration::new(1, 0));
/// let new_now = Instant::now();
/// println!("{:?}", new_now.duration_since(now));
/// println!("{:?}", now.duration_since(new_now)); // 0ns
/// ```
#[must_use]
#[stable(feature = "time2", since = "1.8.0")]
pub fn duration_since(&self, earlier: Instant) -> Duration {
self.0.checked_sub_instant(&earlier.0).expect("supplied instant is later than self")
self.checked_duration_since(earlier).unwrap_or_default()
}
/// Returns the amount of time elapsed from another instant to this one,
/// or None if that instant is later than this one.
///
/// Due to [monotonicity bugs], even under correct logical ordering of the passed `Instant`s,
/// this method can return `None`.
///
/// [monotonicity bugs]: Instant#monotonicity
///
/// # Examples
///
/// ```no_run
@ -362,12 +337,6 @@ pub fn saturating_duration_since(&self, earlier: Instant) -> Duration {
/// Returns the amount of time elapsed since this instant was created.
///
/// # Panics
///
/// This function may panic if the current time is earlier than this
/// instant, which is something that can happen if an `Instant` is
/// produced synthetically.
///
/// # Examples
///
/// ```no_run

View file

@ -1,116 +0,0 @@
use crate::sys::time;
#[inline]
pub(super) fn monotonize(raw: time::Instant) -> time::Instant {
inner::monotonize(raw)
}
#[cfg(any(all(target_has_atomic = "64", not(target_has_atomic = "128")), target_arch = "aarch64"))]
pub mod inner {
use crate::sync::atomic::AtomicU64;
use crate::sync::atomic::Ordering::*;
use crate::sys::time;
use crate::time::Duration;
pub(in crate::time) const ZERO: time::Instant = time::Instant::zero();
// bits 30 and 31 are never used since the nanoseconds part never exceeds 10^9
const UNINITIALIZED: u64 = 0b11 << 30;
static MONO: AtomicU64 = AtomicU64::new(UNINITIALIZED);
#[inline]
pub(super) fn monotonize(raw: time::Instant) -> time::Instant {
monotonize_impl(&MONO, raw)
}
#[inline]
pub(in crate::time) fn monotonize_impl(mono: &AtomicU64, raw: time::Instant) -> time::Instant {
let delta = raw.checked_sub_instant(&ZERO).unwrap();
let secs = delta.as_secs();
// occupies no more than 30 bits (10^9 seconds)
let nanos = delta.subsec_nanos() as u64;
// This wraps around every 136 years (2^32 seconds).
// To detect backsliding we use wrapping arithmetic and declare forward steps smaller
// than 2^31 seconds as expected and everything else as a backslide which will be
// monotonized.
// This could be a problem for programs that call instants at intervals greater
// than 68 years. Interstellar probes may want to ensure that actually_monotonic() is true.
let packed = (secs << 32) | nanos;
let updated = mono.fetch_update(Relaxed, Relaxed, |old| {
(old == UNINITIALIZED || packed.wrapping_sub(old) < u64::MAX / 2).then_some(packed)
});
match updated {
Ok(_) => raw,
Err(newer) => {
// Backslide occurred. We reconstruct monotonized time from the upper 32 bit of the
// passed in value and the 64bits loaded from the atomic
let seconds_lower = newer >> 32;
let mut seconds_upper = secs & 0xffff_ffff_0000_0000;
if secs & 0xffff_ffff > seconds_lower {
// Backslide caused the lower 32bit of the seconds part to wrap.
// This must be the case because the seconds part is larger even though
// we are in the backslide branch, i.e. the seconds count should be smaller or equal.
//
// We assume that backslides are smaller than 2^32 seconds
// which means we need to add 1 to the upper half to restore it.
//
// Example:
// most recent observed time: 0xA1_0000_0000_0000_0000u128
// bits stored in AtomicU64: 0x0000_0000_0000_0000u64
// backslide by 1s
// caller time is 0xA0_ffff_ffff_0000_0000u128
// -> we can fix up the upper half time by adding 1 << 32
seconds_upper = seconds_upper.wrapping_add(0x1_0000_0000);
}
let secs = seconds_upper | seconds_lower;
let nanos = newer as u32;
ZERO.checked_add_duration(&Duration::new(secs, nanos)).unwrap()
}
}
}
}
#[cfg(all(target_has_atomic = "128", not(target_arch = "aarch64")))]
pub mod inner {
use crate::sync::atomic::AtomicU128;
use crate::sync::atomic::Ordering::*;
use crate::sys::time;
use crate::time::Duration;
const ZERO: time::Instant = time::Instant::zero();
static MONO: AtomicU128 = AtomicU128::new(0);
#[inline]
pub(super) fn monotonize(raw: time::Instant) -> time::Instant {
let delta = raw.checked_sub_instant(&ZERO).unwrap();
// Split into seconds and nanos since Duration doesn't have a
// constructor that takes a u128
let secs = delta.as_secs() as u128;
let nanos = delta.subsec_nanos() as u128;
let timestamp: u128 = secs << 64 | nanos;
let timestamp = MONO.fetch_max(timestamp, Relaxed).max(timestamp);
let secs = (timestamp >> 64) as u64;
let nanos = timestamp as u32;
ZERO.checked_add_duration(&Duration::new(secs, nanos)).unwrap()
}
}
#[cfg(not(any(target_has_atomic = "64", target_has_atomic = "128")))]
pub mod inner {
use crate::cmp;
use crate::sys::time;
use crate::sys_common::mutex::StaticMutex;
#[inline]
pub(super) fn monotonize(os_now: time::Instant) -> time::Instant {
static LOCK: StaticMutex = StaticMutex::new();
static mut LAST_NOW: time::Instant = time::Instant::zero();
unsafe {
let _lock = LOCK.lock();
let now = cmp::max(LAST_NOW, os_now);
LAST_NOW = now;
now
}
}
}

View file

@ -90,10 +90,9 @@ fn instant_math_is_associative() {
}
#[test]
#[should_panic]
fn instant_duration_since_panic() {
fn instant_duration_since_saturates() {
let a = Instant::now();
let _ = (a - Duration::SECOND).duration_since(a);
assert_eq!((a - Duration::SECOND).duration_since(a), Duration::ZERO);
}
#[test]
@ -109,6 +108,7 @@ fn instant_checked_duration_since_nopanic() {
#[test]
fn instant_saturating_duration_since_nopanic() {
let a = Instant::now();
#[allow(deprecated, deprecated_in_future)]
let ret = (a - Duration::SECOND).saturating_duration_since(a);
assert_eq!(ret, Duration::ZERO);
}
@ -192,31 +192,6 @@ fn since_epoch() {
assert!(a < hundred_twenty_years);
}
#[cfg(all(target_has_atomic = "64", not(target_has_atomic = "128")))]
#[test]
fn monotonizer_wrapping_backslide() {
use super::monotonic::inner::{monotonize_impl, ZERO};
use core::sync::atomic::AtomicU64;
let reference = AtomicU64::new(0);
let time = match ZERO.checked_add_duration(&Duration::from_secs(0xffff_ffff)) {
Some(time) => time,
None => {
// platform cannot represent u32::MAX seconds so it won't have to deal with this kind
// of overflow either
return;
}
};
let monotonized = monotonize_impl(&reference, time);
let expected = ZERO.checked_add_duration(&Duration::from_secs(1 << 32)).unwrap();
assert_eq!(
monotonized, expected,
"64bit monotonizer should handle overflows in the seconds part"
);
}
macro_rules! bench_instant_threaded {
($bench_name:ident, $thread_count:expr) => {
#[bench]