Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Feb 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions library/std/src/sys/hermit/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,6 @@ impl 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()
}
Expand Down
9 changes: 0 additions & 9 deletions library/std/src/sys/itron/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,6 @@ impl 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
Expand Down
8 changes: 0 additions & 8 deletions library/std/src/sys/sgx/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@ impl 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 {
Expand Down
19 changes: 0 additions & 19 deletions library/std/src/sys/unix/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,6 @@ mod inner {
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();
Expand Down Expand Up @@ -296,17 +288,6 @@ mod inner {
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()
}
Expand Down
8 changes: 0 additions & 8 deletions library/std/src/sys/unsupported/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@ impl 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)
}
Expand Down
8 changes: 0 additions & 8 deletions library/std/src/sys/wasi/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@ impl 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)
}
Expand Down
8 changes: 0 additions & 8 deletions library/std/src/sys/windows/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@ impl 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,
Expand Down
109 changes: 54 additions & 55 deletions library/std/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@

#![stable(feature = "time", since = "1.3.0")]

mod monotonic;
#[cfg(test)]
mod tests;

Expand All @@ -50,8 +49,8 @@ pub use core::time::FromFloatSecsError;
/// A measurement of a monotonically nondecreasing clock.
/// Opaque and useful only with [`Duration`].
///
/// Instants are always guaranteed to be no less than any previously measured
/// instant when created, and are often useful for tasks such as measuring
/// Instants are always guaranteed, barring [platform bugs], to be no less than any previously
/// measured instant when created, and are often useful for tasks such as measuring
/// benchmarks or timing how long an operation takes.
///
/// Note, however, that instants are **not** guaranteed to be **steady**. In other
Expand Down Expand Up @@ -84,6 +83,8 @@ pub use core::time::FromFloatSecsError;
/// }
/// ```
///
/// [platform bugs]: Instant#monotonicity
///
/// # OS-specific behaviors
///
/// An `Instant` is a wrapper around system-specific types and it may behave
Expand Down Expand Up @@ -125,6 +126,26 @@ pub use core::time::FromFloatSecsError;
/// > 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. In older Rust versions this
/// lead to a panic instead. [`checked_duration_since`] can be used to detect and handle situations
/// where monotonicity is violated, or `Instant`s are subtracted in the wrong order.
///
/// This workaround obscures programming errors where earlier and later instants are accidentally
/// swapped. For this reason future rust versions may reintroduce panics.
///
/// [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);
Expand Down Expand Up @@ -247,59 +268,19 @@ 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`.
/// Previous rust versions panicked when `earlier` was later than `self`. Currently this
/// method saturates. Future versions may reintroduce the panic in some circumstances.
/// See [Monotonicity].
///
/// [Monotonicity]: Instant#monotonicity
///
/// # Examples
///
Expand All @@ -311,16 +292,22 @@ impl 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
Expand Down Expand Up @@ -364,9 +351,11 @@ impl Instant {
///
/// # 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.
/// Previous rust versions panicked when self was earlier than the current time. Currently this
/// method returns a Duration of zero in that case. Future versions may reintroduce the panic.
/// See [Monotonicity].
///
/// [Monotonicity]: Instant#monotonicity
///
/// # Examples
///
Expand Down Expand Up @@ -442,6 +431,16 @@ impl SubAssign<Duration> for Instant {
impl Sub<Instant> for Instant {
type Output = Duration;

/// Returns the amount of time elapsed from another instant to this one,
/// or zero duration if that instant is later than this one.
///
/// # Panics
///
/// Previous rust versions panicked when `other` was later than `self`. Currently this
/// method saturates. Future versions may reintroduce the panic in some circumstances.
/// See [Monotonicity].
///
/// [Monotonicity]: Instant#monotonicity
fn sub(self, other: Instant) -> Duration {
self.duration_since(other)
}
Expand Down
Loading