Initial implementation of Stopwatch - #161817
Conversation
rustbot
added
the
S-waiting-on-review
rustbot
added
the
T-libs
f1788d9 to
8c3406c
Compare
|
Huh, I'm not actually sure about apple. I thought |
7983c80 to
2c64ef2
Compare
|
@bors try jobs=aarch64-apple-* |
This comment has been minimized.
This comment has been minimized.
Initial implementation of `Stopwatch` try-job: aarch64-apple-*
This comment has been minimized.
This comment has been minimized.
It is monotonic, but it's measured by subtracting the system boot time from |
|
|
||
| impl PartialEq for Stopwatch { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| self.ticks.abs_diff(other.ticks) <= 1 |
There was a problem hiding this comment.
This violates transitivity: Stopwatch { ticks: 0 } is equal to Stopwatch { ticks: 1 }, which in turn equals Stopwatch { ticks: 2 }, but Stopwatch { ticks: 0 } is not equal to Stopwatch { ticks: 2 }.
There was a problem hiding this comment.
Right. I was trying to simplify it and went too far, ha.
There was a problem hiding this comment.
I'd move the Instant-wrapping implementation here instead of duplicating it across all those platforms.
There was a problem hiding this comment.
Sure. I was anticipating them diverging more once more platform maintainers start poking at it but a common default implementation is useful regardless.
| #[repr(C)] | ||
| struct mach_timebase_info { | ||
| numer: u32, | ||
| denom: u32, | ||
| } | ||
|
|
||
| unsafe extern "C" { | ||
| unsafe fn mach_timebase_info(info: *mut mach_timebase_info) -> libc::kern_return_t; | ||
| } | ||
| let mut timebase = mach_timebase_info { numer: 0, denom: 0 }; | ||
| assert_eq!(unsafe { mach_timebase_info(&mut timebase) }, libc::KERN_SUCCESS); |
There was a problem hiding this comment.
This logic already exists for Instant::into_mach_absolute_time_ceil (it's used for sleep_until), it'd be nice to avoid duplication.
| /// | SGX | [`insecure_time` usercall]. More information on [timekeeping in SGX] | | ||
| /// | UNIX | [clock_gettime] with `CLOCK_MONOTONIC` | | ||
| /// | WASI | [clock_gettime] with `CLOCK_MONOTONIC` | | ||
| /// | Darwin | [clock_gettime] with `CLOCK_MONOTONIC` | |
There was a problem hiding this comment.
This will need updating...
| /// The following system calls are [currently] being used by `now()` to find out | ||
| /// the current time: | ||
| /// | ||
| /// | Platform | System call | |
There was a problem hiding this comment.
I wonder whether it'd be easier to only mention the platforms where Stopwatch deviates from Instant.
There was a problem hiding this comment.
I think if/when this is stable it'd be good to consulate some of these docs at the module level. But in the meantime I guess it makes sense to keep duplication to a minimum.
There was a problem hiding this comment.
Oh yeah, module-level sounds great! I've just noticed a tendency for these tables to fall out of sync with the actual implementation, so keeping duplication to a minimum hopefully helps prevent that.
rustbot
added
S-waiting-on-author
4 participants
For most platforms this currently delegates to
Instantbut they could drift apart later. For Linux I consideredCLOCK_MONOTONIC_RAWbut I wasn't fully convinced it was the right thing to do since adjustments do makeCLOCK_MONOTONICmore accurate (and apparently there's a bug in older kernels that make calls toCLOCK_MONOTONIC_RAWslower). Windows currently usesQueryPerformanceCounterfor bothInstantandStopwatchbut without the need to convert to a duration as soon as getting the counter. It is expected forInstantto change in the future to use the same clock as timeouts, etc.Tracking issue: #161809
r? joboet