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

Use CLOCK_MONOTONIC_COARSE in libnative on linux. #11981

Closed
wants to merge 1 commit into from
Closed

Use CLOCK_MONOTONIC_COARSE in libnative on linux. #11981

wants to merge 1 commit into from

Conversation

bnoordhuis
Copy link
Contributor

Or CLOCK_MONOTONIC if CLOCK_MONOTONIC_COARSE isn't available but prefer
CLOCK_MONOTONIC_COARSE. The normal monotonic clock is fetched using a
system call but the coarse clock is read directly from the vDSO and
hence is a lot faster.

Make sure it's high-res enough: it normally has a 1 kHz frequency on
systems tuned for interactivity but on systems that are tuned for
throughput, it may run as slow as 100 Hz.

Whether the normal or the coarse clock is used, it should be an
improvement either way. Before, libnative called gettimeofday() and
that polls a clock that is subject to clock skew.

gettimeofday() is still the time source on the other UNIX platforms,
something that will eventually need to be addressed.

Or CLOCK_MONOTONIC if CLOCK_MONOTONIC_COARSE isn't available but prefer
CLOCK_MONOTONIC_COARSE.  The normal monotonic clock is fetched using a
system call but the coarse clock is read directly from the vDSO and
hence is a lot faster.

Make sure it's high-res enough: it normally has a 1 kHz frequency on
systems tuned for interactivity but on systems that are tuned for
throughput, it may run as slow as 100 Hz.

Whether the normal or the coarse clock is used, it should be an
improvement either way.  Before, libnative called gettimeofday() and
that polls a clock that is subject to clock skew.

gettimeofday() is still the time source on the other UNIX platforms,
something that will eventually need to be addressed.
@thestinger
Copy link
Contributor

clock_gettime(CLOCK_MONOTONIC) also uses the VDSO, although I think it does a bit more work and there's no reason to not use the coarse one here. Both are actually subject to clock skew (time slowed or sped up) on Linux but will never jump backwards or forwards. The CLOCK_MONOTONIC_RAW function isn't subject to clock skew but skips the VDSO and may run too slow or too fast on hardware where the CPU's timer isn't great (thus, skew).

@alexcrichton
Copy link
Member

This is an interesting idea, but sadly I don't think the code will do much as-is. The timer_other.rs file is currently only compiled for osx/freebsd/android, so the code in question won't actually be used (linux uses timer_timerfd.rs).

We do have another "precise timer" in src/libextra/time.rs which is indeed compiled for linux. I personally don't know much about the array of clocks that linux provides, but #11233 may be of interest (lots of discussion over there).

@bnoordhuis
Copy link
Contributor Author

Ah, sorry about that. I checked for clock_getres() and clock_gettime() calls afterwards with strace but those probably come from somewhere else then.

clock_gettime(CLOCK_MONOTONIC) also uses the VDSO, although I think it does a bit more work and there's no reason to not use the coarse one here.

That's true insofar that clock_gettime(CLOCK_MONOTONIC) starts out as a call to __vdso_clock_gettime() but that function falls back to making a system call when there is no reliable hardware clock to read from (most virtualized systems.)

And as you say, it does more work. It's also very expensive when running a non-paravirt kernel under a hypervisor because the hypervisor traps the rdtsc instruction. Anyway, I suppose that's all academic for now. :-)

@bnoordhuis bnoordhuis closed this Feb 2, 2014
@bnoordhuis bnoordhuis deleted the clock-monotonic-coarse branch February 2, 2014 11:44
@thestinger
Copy link
Contributor

Is the now function just used for relative timers? If that's the only use, then it should certainly use one of the monotonic clocks. I don't know if it should use the coarse clock or the regular one, but it appears that with the current interface it can use the coarse one without a loss.

@thestinger
Copy link
Contributor

(sorry for the noise, I just woke up! :P)

@bnoordhuis
Copy link
Contributor Author

Is the now function just used for relative timers? If that's the only use, then it should certainly use one of the monotonic clocks.

Yes, agreed. This PR was me working up to a cross-platform solution with some Linux-only performance optimizations thrown in for the sake of it. I'll try to revisit it later this week.

flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 28, 2023
new lint: `eager_transmute`

A small but still hopefully useful lint that looks for patterns such as `(x < 5).then_some(transmute(x))`.
This is almost certainly wrong because it evaluates the transmute eagerly and can lead to surprises such as the check being completely removed and always evaluating to `Some` no matter what `x` is (it is UB after all when the integer is not a valid bitpattern for the transmuted-to type). [Example](https://godbolt.org/z/xoY34fPzh).
The user most likely meant to use `then` instead.

I can't remember where I saw this but this is inspired by a real bug that happened in practice.

This could probably be a correctness lint?

changelog: new lint: [`eager_int_transmute`]
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 11, 2024
…ogiq

Lint nested binary operations and handle field projections in `eager_transmute`

This PR makes the lint a bit stronger. Previously it would only lint `(x < 4).then_some(transmute(x))` (that is, a single binary op in the condition). With this change, it understands:
- multiple, nested binary ops: `(x < 4 && x > 1).then_some(...)`
- local references with projections: `(x.field < 4 && x.field > 1).then_some(transmute(x.field))`

changelog: [`eager_transmute`]: lint nested binary operations and look through field/array accesses

r? llogiq (since you reviewed my initial PR rust-lang#11981, I figured you have the most context here, sorry if you are too busy with other PRs, feel free to reassign to someone else then)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants