Skip to content

Commit

Permalink
fix clobbering of global recent time making tests flaky + formatting
Browse files Browse the repository at this point in the history
  • Loading branch information
tobz committed Mar 24, 2023
1 parent 2eb5cd2 commit a4f2055
Showing 1 changed file with 39 additions and 16 deletions.
55 changes: 39 additions & 16 deletions src/instant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ use std::time::Duration;
///
/// ## Monotonicity
///
/// On all platforms `Instant` will try to use an OS API that guarantees monotonic behavior
/// if available, which is the case for all supported 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 `quanta` 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.
/// On all platforms, `Instant` will try to use an OS API that guarantees monotonic behavior if
/// available, which is the case for all supported 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 `quanta` 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 `quanta` versions may reintroduce panics.
Expand All @@ -25,7 +25,6 @@ use std::time::Duration;
/// [`elapsed`]: Instant::elapsed
/// [`sub`]: Instant::sub
/// [`checked_duration_since`]: Instant::checked_duration_since
///
#[derive(Clone, Copy, PartialEq, Eq)]
pub struct Instant(pub(crate) u64);

Expand Down Expand Up @@ -66,9 +65,9 @@ impl Instant {
///
/// # Panics
///
/// Previous `quanta` versions panicked when `earlier` was later than `self`. Currently this
/// method saturates. Future versions may reintroduce the panic in some circumstances.
/// See [Monotonicity].
/// Previous versions of this method panicked when earlier was later than `self`. Currently,
/// this method saturates to zero. Future versions may reintroduce the panic in some
/// circumstances. See [Monotonicity].
///
/// [Monotonicity]: Instant#monotonicity
///
Expand Down Expand Up @@ -118,6 +117,11 @@ impl Instant {
/// Returns the amount of time elapsed from another instant to this one, or zero duration if
/// that instant is earlier 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 All @@ -140,9 +144,9 @@ impl Instant {
///
/// # Panics
///
/// Previous `quanta` versions panicked when the current time was earlier than self.
/// Currently this method returns a Duration of zero in that case.
/// Future versions may reintroduce the panic. See [Monotonicity].
/// Previous `quanta` versions panicked when the current time was earlier than self. Currently
/// this method returns a Duration of zero in that case. Future versions may reintroduce the
/// panic. See [Monotonicity].
///
/// [Monotonicity]: Instant#monotonicity
///
Expand Down Expand Up @@ -273,17 +277,23 @@ impl Into<prost_types::Timestamp> for Instant {

#[cfg(test)]
mod tests {
use once_cell::sync::Lazy;

use super::Instant;
use crate::{with_clock, Clock};
use std::thread;
use std::time::Duration;
use std::{sync::Mutex, thread};

static RECENT_LOCK: Lazy<Mutex<()>> = Lazy::new(|| Mutex::new(()));

#[test]
#[cfg_attr(
all(target_arch = "wasm32", target_os = "unknown"),
ignore = "WASM thread cannot sleep"
)]
fn test_now() {
let _guard = RECENT_LOCK.lock().unwrap();

let t0 = Instant::now();
thread::sleep(Duration::from_millis(15));
let t1 = Instant::now();
Expand All @@ -302,6 +312,8 @@ mod tests {
ignore = "WASM thread cannot sleep"
)]
fn test_recent() {
let _guard = RECENT_LOCK.lock().unwrap();

// Ensures that the recent global value is zero so that the fallback logic can kick in.
crate::set_recent(Instant(0));

Expand All @@ -314,7 +326,13 @@ mod tests {

let result = t1 - t0;
let threshold = Duration::from_millis(14);
assert!(result > threshold);
assert!(
result > threshold,
"t1 should be greater than t0 by at least 14ms, was only {}ms (t0: {}, t1: {})",
result.as_millis(),
t0.0,
t1.0
);

crate::set_recent(Instant(1));
let t2 = Instant::recent();
Expand All @@ -329,6 +347,11 @@ mod tests {
wasm_bindgen_test::wasm_bindgen_test
)]
fn test_mocking() {
let _guard = RECENT_LOCK.lock().unwrap();

// Ensures that the recent global value is zero so that the fallback logic can kick in.
crate::set_recent(Instant(0));

let (clock, mock) = Clock::mock();
with_clock(&clock, move || {
let t0 = Instant::now();
Expand Down

2 comments on commit a4f2055

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bench-ubuntu-latest

Benchmark suite Current: a4f2055 Previous: 2eb5cd2 Ratio
stdlib/now/1 25 ns/iter (± 0) 26 ns/iter (± 0) 0.96
stdlib/now/2 26 ns/iter (± 2) 27 ns/iter (± 0) 0.96
stdlib/now/3 39 ns/iter (± 6) 39 ns/iter (± 4) 1
stdlib/now/4 49 ns/iter (± 7) 52 ns/iter (± 7) 0.94
stdlib/now/5 64 ns/iter (± 12) 64 ns/iter (± 12) 1
stdlib/now/6 78 ns/iter (± 17) 77 ns/iter (± 16) 1.01
stdlib/now/7 85 ns/iter (± 21) 88 ns/iter (± 17) 0.97
stdlib/now/8 95 ns/iter (± 23) 101 ns/iter (± 24) 0.94
stdlib/now/9 106 ns/iter (± 28) 111 ns/iter (± 25) 0.95
stdlib/now/10 133 ns/iter (± 33) 123 ns/iter (± 33) 1.08
stdlib/now/11 124 ns/iter (± 36) 132 ns/iter (± 35) 0.94
stdlib/now/12 144 ns/iter (± 37) 139 ns/iter (± 36) 1.04
quanta/now/1 28 ns/iter (± 1) 30 ns/iter (± 0) 0.93
quanta/now/2 31 ns/iter (± 2) 30 ns/iter (± 0) 1.03
quanta/now/3 43 ns/iter (± 5) 45 ns/iter (± 5) 0.96
quanta/now/4 57 ns/iter (± 9) 58 ns/iter (± 9) 0.98
quanta/now/5 71 ns/iter (± 16) 73 ns/iter (± 13) 0.97
quanta/now/6 97 ns/iter (± 20) 87 ns/iter (± 17) 1.11
quanta/now/7 95 ns/iter (± 19) 100 ns/iter (± 20) 0.95
quanta/now/8 109 ns/iter (± 25) 114 ns/iter (± 25) 0.96
quanta/now/9 125 ns/iter (± 29) 129 ns/iter (± 30) 0.97
quanta/now/10 138 ns/iter (± 30) 140 ns/iter (± 35) 0.99
quanta/now/11 150 ns/iter (± 45) 150 ns/iter (± 42) 1
quanta/now/12 161 ns/iter (± 38) 160 ns/iter (± 38) 1.01
stdlib/instant_now 26 ns/iter (± 1) 27 ns/iter (± 0) 0.96
stdlib/instant_delta 55 ns/iter (± 2) 60 ns/iter (± 0) 0.92
quanta/quanta_now 25 ns/iter (± 2) 26 ns/iter (± 0) 0.96
quanta/quanta_now_delta 56 ns/iter (± 2) 60 ns/iter (± 0) 0.93
quanta/quanta_instant_now 29 ns/iter (± 1) 30 ns/iter (± 0) 0.97
quanta/quanta_raw 25 ns/iter (± 1) 26 ns/iter (± 0) 0.96
quanta/quanta_raw_scaled 26 ns/iter (± 1) 27 ns/iter (± 0) 0.96
quanta/quanta_raw_delta 56 ns/iter (± 3) 56 ns/iter (± 0) 1
quanta/quanta_recent 1 ns/iter (± 0) 2 ns/iter (± 0) 0.50
quanta/quanta_instant_recent 1 ns/iter (± 0) 1 ns/iter (± 0) 1

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'bench-ubuntu-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 0.1.

Benchmark suite Current: a4f2055 Previous: 2eb5cd2 Ratio
stdlib/now/1 25 ns/iter (± 0) 26 ns/iter (± 0) 0.96
stdlib/now/2 26 ns/iter (± 2) 27 ns/iter (± 0) 0.96
stdlib/now/3 39 ns/iter (± 6) 39 ns/iter (± 4) 1
stdlib/now/4 49 ns/iter (± 7) 52 ns/iter (± 7) 0.94
stdlib/now/5 64 ns/iter (± 12) 64 ns/iter (± 12) 1
stdlib/now/6 78 ns/iter (± 17) 77 ns/iter (± 16) 1.01
stdlib/now/7 85 ns/iter (± 21) 88 ns/iter (± 17) 0.97
stdlib/now/8 95 ns/iter (± 23) 101 ns/iter (± 24) 0.94
stdlib/now/9 106 ns/iter (± 28) 111 ns/iter (± 25) 0.95
stdlib/now/10 133 ns/iter (± 33) 123 ns/iter (± 33) 1.08
stdlib/now/11 124 ns/iter (± 36) 132 ns/iter (± 35) 0.94
stdlib/now/12 144 ns/iter (± 37) 139 ns/iter (± 36) 1.04
quanta/now/1 28 ns/iter (± 1) 30 ns/iter (± 0) 0.93
quanta/now/2 31 ns/iter (± 2) 30 ns/iter (± 0) 1.03
quanta/now/3 43 ns/iter (± 5) 45 ns/iter (± 5) 0.96
quanta/now/4 57 ns/iter (± 9) 58 ns/iter (± 9) 0.98
quanta/now/5 71 ns/iter (± 16) 73 ns/iter (± 13) 0.97
quanta/now/6 97 ns/iter (± 20) 87 ns/iter (± 17) 1.11
quanta/now/7 95 ns/iter (± 19) 100 ns/iter (± 20) 0.95
quanta/now/8 109 ns/iter (± 25) 114 ns/iter (± 25) 0.96
quanta/now/9 125 ns/iter (± 29) 129 ns/iter (± 30) 0.97
quanta/now/10 138 ns/iter (± 30) 140 ns/iter (± 35) 0.99
quanta/now/11 150 ns/iter (± 45) 150 ns/iter (± 42) 1
quanta/now/12 161 ns/iter (± 38) 160 ns/iter (± 38) 1.01
stdlib/instant_now 26 ns/iter (± 1) 27 ns/iter (± 0) 0.96
stdlib/instant_delta 55 ns/iter (± 2) 60 ns/iter (± 0) 0.92
quanta/quanta_now 25 ns/iter (± 2) 26 ns/iter (± 0) 0.96
quanta/quanta_now_delta 56 ns/iter (± 2) 60 ns/iter (± 0) 0.93
quanta/quanta_instant_now 29 ns/iter (± 1) 30 ns/iter (± 0) 0.97
quanta/quanta_raw 25 ns/iter (± 1) 26 ns/iter (± 0) 0.96
quanta/quanta_raw_scaled 26 ns/iter (± 1) 27 ns/iter (± 0) 0.96
quanta/quanta_raw_delta 56 ns/iter (± 3) 56 ns/iter (± 0) 1
quanta/quanta_recent 1 ns/iter (± 0) 2 ns/iter (± 0) 0.50
quanta/quanta_instant_recent 1 ns/iter (± 0) 1 ns/iter (± 0) 1

This comment was automatically generated by workflow using github-action-benchmark.

CC: @tobz

Please sign in to comment.