Skip to content

Use CLOCK_MONOTONIC_RAW for Instant when available #37907

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

Closed
wants to merge 1 commit into from

Conversation

sfackler
Copy link
Member

Closes #37902

r? @alexcrichton

@nagisa
Copy link
Member

nagisa commented Nov 21, 2016

I find it dubious to work around a bug in an ancient version of the kernel by using MONOTONIC_RAW.

@sfackler
Copy link
Member Author

CLOCK_MONOTONIC_RAW is a more appropriate clock for Instant regardless since it isn't affected by NTP adjustments.

@Amanieu
Copy link
Member

Amanieu commented Nov 21, 2016

@sfackler See my comment in #37902. We actually do want to take NTP adjustments into account since we want our clock to stay in sync with real time (1 second on clock = 1 real second).

@alexcrichton
Copy link
Member

Some thoughts of mine:

So in light of that I like this approach as it works around buggy kernels in what appear to be somewhat common circumstances. In that sense I think the only question is for systems which have non-buggy versions of both clocks whether we use the raw versions or not (basically @Amanieu's question). I would tend to agree that CLOCK_MONOTONIC seems like the most accurate here in terms of what we'd like to have, although I believe the documentation for Instant would allow both.

Is it possible for us to isolate these buggy kernels? Can they somehow be detected? Do we know for sure that it was a buggy kernel and can track down versions? If we can do that then we could perhaps use the raw clock only on these buggy kernels.

@Amanieu
Copy link
Member

Amanieu commented Nov 21, 2016

We could detect buggy kernels by caching the last value returned by clock_gettime(CLOCK_MONOTONIC) and checking if a newly return value is earlier than the cached value. However this has performance implications since you would need a mutex to serialize calls to clock_gettime, which may not be desirable since this functionality is often used for fine-grained timing (e.g. benchmarking).

@sfackler
Copy link
Member Author

It does depend on your use case - at low durations you'd preseumably prefer the slight inaccuracy of the raw clock over the possibility of measuring inside of an NTP-induced skew.

Seems reasonable that CLOCK_MONOTONIC is a better default choice though.

@sfackler sfackler closed this Nov 21, 2016
@sfackler sfackler deleted the monotonic-raw branch November 21, 2016 17:03
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.

4 participants