Skip to content

Commit 9764df5

Browse files
authored
Merge pull request #481 from moka-rs/increase-time-accuracy
Replace most uses of `quanta::Instant` with `std::time::Instant` to increase the accuracy of time measurements
2 parents 5dbd7ab + 5d03e02 commit 9764df5

21 files changed

+540
-675
lines changed

CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
### Changed
66

7+
- Replaced most uses of `quanta::Instant` with `std::time::Instant` to increase the
8+
accuracy of time measurements. ([#481][gh-pull-0481])
79
- Switched to `AtomicU64` of `portable-atomic` crate for the platforms where
810
`AtomicU64` is not available in `std`. ([#480][gh-pull-0480])
911
- `moka`'s `atomic64` feature no longer has any effect on the build as
@@ -948,6 +950,7 @@ The minimum supported Rust version (MSRV) is now 1.51.0 (Mar 25, 2021).
948950
[gh-issue-0034]: https://github.com/moka-rs/moka/issues/34/
949951
[gh-issue-0031]: https://github.com/moka-rs/moka/issues/31/
950952

953+
[gh-pull-0481]: https://github.com/moka-rs/moka/pull/481/
951954
[gh-pull-0480]: https://github.com/moka-rs/moka/pull/480/
952955
[gh-pull-0474]: https://github.com/moka-rs/moka/pull/474/
953956
[gh-pull-0466]: https://github.com/moka-rs/moka/pull/466/

src/common/concurrent.rs

-7
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,6 @@ pub(crate) mod entry_info;
1212
#[cfg(feature = "sync")]
1313
pub(crate) mod housekeeper;
1414

15-
#[cfg_attr(feature = "quanta", path = "concurrent/atomic_time/atomic_time.rs")]
16-
#[cfg_attr(
17-
not(feature = "quanta"),
18-
path = "concurrent/atomic_time/atomic_time_compat.rs"
19-
)]
20-
pub(crate) mod atomic_time;
21-
2215
#[cfg(feature = "unstable-debug-counters")]
2316
pub(crate) mod debug_counters;
2417

src/common/concurrent/atomic_time/atomic_time.rs

-61
This file was deleted.

src/common/concurrent/atomic_time/atomic_time_compat.rs

-40
This file was deleted.

src/common/concurrent/entry_info.rs

+5-10
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::sync::atomic::{self, AtomicBool, AtomicU16, AtomicU32, Ordering};
22

33
use super::{AccessTime, KeyHash};
4-
use crate::common::{concurrent::atomic_time::AtomicInstant, time::Instant};
4+
use crate::common::time::{AtomicInstant, Instant};
55

66
#[derive(Debug)]
77
pub(crate) struct EntryInfo<K> {
@@ -191,7 +191,6 @@ mod test {
191191
// e.g. "1.64"
192192
let ver =
193193
option_env!("RUSTC_SEMVER").expect("RUSTC_SEMVER env var was not set at compile time");
194-
let is_quanta_enabled = cfg!(feature = "quanta");
195194
let arch = if cfg!(target_os = "linux") {
196195
if cfg!(target_pointer_width = "64") {
197196
Linux64
@@ -214,14 +213,10 @@ mod test {
214213
panic!("Unsupported target architecture");
215214
};
216215

217-
let expected_sizes = match (arch, is_quanta_enabled) {
218-
(Linux64 | Linux32Arm | Linux32Mips, true) => vec![("1.51", 56)],
219-
(Linux32X86, true) => vec![("1.51", 48)],
220-
(MacOS64, true) => vec![("1.62", 56)],
221-
(Linux64, false) => vec![("1.66", 104), ("1.60", 128)],
222-
(Linux32X86, false) => unimplemented!(),
223-
(Linux32Arm | Linux32Mips, false) => vec![("1.66", 104), ("1.62", 128), ("1.60", 80)],
224-
(MacOS64, false) => vec![("1.62", 104)],
216+
let expected_sizes = match arch {
217+
Linux64 | Linux32Arm | Linux32Mips => vec![("1.51", 56)],
218+
Linux32X86 => vec![("1.51", 48)],
219+
MacOS64 => vec![("1.62", 56)],
225220
};
226221

227222
let mut expected = None;

src/common/concurrent/housekeeper.rs

+9-15
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
use super::constants::LOG_SYNC_INTERVAL_MILLIS;
22

3-
use super::{
4-
atomic_time::AtomicInstant,
5-
constants::{READ_LOG_FLUSH_POINT, WRITE_LOG_FLUSH_POINT},
6-
};
7-
use crate::common::time::{CheckedTimeOps, Instant};
3+
use super::constants::{READ_LOG_FLUSH_POINT, WRITE_LOG_FLUSH_POINT};
4+
use crate::common::time::{AtomicInstant, Instant};
85
use crate::common::HousekeeperConfig;
96

107
use parking_lot::{Mutex, MutexGuard};
@@ -52,7 +49,11 @@ pub(crate) struct Housekeeper {
5249
}
5350

5451
impl Housekeeper {
55-
pub(crate) fn new(is_eviction_listener_enabled: bool, config: HousekeeperConfig) -> Self {
52+
pub(crate) fn new(
53+
is_eviction_listener_enabled: bool,
54+
config: HousekeeperConfig,
55+
now: Instant,
56+
) -> Self {
5657
let (more_entries_to_evict, maintenance_task_timeout) = if is_eviction_listener_enabled {
5758
(
5859
Some(AtomicBool::new(false)),
@@ -64,7 +65,7 @@ impl Housekeeper {
6465

6566
Self {
6667
run_lock: Mutex::default(),
67-
run_after: AtomicInstant::new(Self::sync_after(Instant::now())),
68+
run_after: AtomicInstant::new(Self::sync_after(now)),
6869
more_entries_to_evict,
6970
maintenance_task_timeout,
7071
max_log_sync_repeats: config.max_log_sync_repeats,
@@ -127,10 +128,7 @@ impl Housekeeper {
127128

128129
fn sync_after(now: Instant) -> Instant {
129130
let dur = Duration::from_millis(LOG_SYNC_INTERVAL_MILLIS);
130-
let ts = now.checked_add(dur);
131-
// Assuming that `now` is current wall clock time, this should never fail at
132-
// least next millions of years.
133-
ts.expect("Timestamp overflow")
131+
now.saturating_add(dur)
134132
}
135133
}
136134

@@ -139,8 +137,4 @@ impl Housekeeper {
139137
pub(crate) fn disable_auto_run(&self) {
140138
self.auto_run_enabled.store(false, Ordering::Relaxed);
141139
}
142-
143-
pub(crate) fn reset_run_after(&self, now: Instant) {
144-
self.run_after.set_instant(Self::sync_after(now));
145-
}
146140
}

src/common/time.rs

+5-48
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,10 @@
1-
use std::time::Duration;
2-
3-
#[cfg_attr(feature = "quanta", path = "time/clock_quanta.rs")]
4-
#[cfg_attr(not(feature = "quanta"), path = "time/clock_compat.rs")]
5-
pub(crate) mod clock;
1+
mod atomic_time;
2+
mod clock;
3+
mod instant;
64

5+
pub(crate) use atomic_time::AtomicInstant;
76
pub(crate) use clock::Clock;
7+
pub(crate) use instant::Instant;
88

99
#[cfg(test)]
1010
pub(crate) use clock::Mock;
11-
12-
/// a wrapper type over Instant to force checked additions and prevent
13-
/// unintentional overflow. The type preserve the Copy semantics for the wrapped
14-
#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Debug)]
15-
pub(crate) struct Instant(clock::Instant);
16-
17-
pub(crate) trait CheckedTimeOps {
18-
fn checked_add(&self, duration: Duration) -> Option<Self>
19-
where
20-
Self: Sized;
21-
22-
fn checked_duration_since(&self, earlier: Self) -> Option<Duration>
23-
where
24-
Self: Sized;
25-
}
26-
27-
impl Instant {
28-
pub(crate) fn new(instant: clock::Instant) -> Instant {
29-
Instant(instant)
30-
}
31-
32-
pub(crate) fn now() -> Instant {
33-
Instant(clock::Instant::now())
34-
}
35-
36-
#[cfg(feature = "quanta")]
37-
pub(crate) fn inner_clock(self) -> clock::Instant {
38-
self.0
39-
}
40-
}
41-
42-
impl CheckedTimeOps for Instant {
43-
fn checked_add(&self, duration: Duration) -> Option<Instant> {
44-
self.0.checked_add(duration).map(Instant)
45-
}
46-
47-
fn checked_duration_since(&self, earlier: Self) -> Option<Duration>
48-
where
49-
Self: Sized,
50-
{
51-
self.0.checked_duration_since(earlier.0)
52-
}
53-
}

src/common/time/atomic_time.rs

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
use crate::common::time::Instant;
2+
3+
use portable_atomic::AtomicU64;
4+
use std::sync::atomic::Ordering;
5+
6+
/// `AtomicInstant` is a wrapper around `AtomicU64` that provides thread-safe access
7+
/// to an `Instant`.
8+
///
9+
/// `u64::MAX` is used to represent an unset `Instant`.
10+
#[derive(Debug)]
11+
pub(crate) struct AtomicInstant {
12+
instant: AtomicU64,
13+
}
14+
15+
impl Default for AtomicInstant {
16+
/// Creates a new `AtomicInstant` with an unset `Instant`.
17+
fn default() -> Self {
18+
Self {
19+
instant: AtomicU64::new(u64::MAX),
20+
}
21+
}
22+
}
23+
24+
impl AtomicInstant {
25+
/// Creates a new `AtomicInstant` with the given `Instant`.
26+
pub(crate) fn new(instant: Instant) -> Self {
27+
// Ensure the `Instant` is not `u64::MAX`, which means unset.
28+
debug_assert!(instant.as_nanos() != u64::MAX);
29+
30+
Self {
31+
instant: AtomicU64::new(instant.as_nanos()),
32+
}
33+
}
34+
35+
/// Clears the `Instant`.
36+
pub(crate) fn clear(&self) {
37+
self.instant.store(u64::MAX, Ordering::Release);
38+
}
39+
40+
/// Returns `true` if the `Instant` is set.
41+
pub(crate) fn is_set(&self) -> bool {
42+
self.instant.load(Ordering::Acquire) != u64::MAX
43+
}
44+
45+
/// Returns the `Instant` if it is set, otherwise `None`.
46+
pub(crate) fn instant(&self) -> Option<Instant> {
47+
let ts = self.instant.load(Ordering::Acquire);
48+
if ts == u64::MAX {
49+
None
50+
} else {
51+
Some(Instant::from_nanos(ts))
52+
}
53+
}
54+
55+
/// Sets the `Instant`.
56+
pub(crate) fn set_instant(&self, instant: Instant) {
57+
// Ensure the `Instant` is not `u64::MAX`, which means unset.
58+
debug_assert!(instant.as_nanos() != u64::MAX);
59+
60+
self.instant.store(instant.as_nanos(), Ordering::Release);
61+
}
62+
}

0 commit comments

Comments
 (0)