Skip to content

use a monotonic clock for timeouts #253

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

Merged
merged 1 commit into from
Feb 24, 2015

Conversation

tenderlove
Copy link
Contributor

We should use a monotonic clock for timeouts so that if the time changes
on the system it will not impact the timeout code.


if defined?(Process::CLOCK_MONOTONIC)
def clock_time
Process.clock_time Process::CLOCK_MONOTONIC
Copy link
Member

Choose a reason for hiding this comment

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

@tenderlove Should this method be clock_gettime?

This is a Ruby feature I wasn't aware of until you created this PR so thank you for pointing it out. This solves a problem we've discussed in another issue. A ton of tests are failing, however. Should the method call be this? http://ruby-doc.org//core-2.2.0/Process.html#method-c-clock_gettime

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it should be. That's what I get for not running the tests. >_<

I'll update the patch

We should use a monotonic clock for timeouts so that if the time changes
on the system it will not impact the timeout code.
@jdantonio
Copy link
Member

No sweat. Our tests take a long time to run. Even I skip them occasionally!

jdantonio added a commit that referenced this pull request Feb 24, 2015
use a monotonic clock for timeouts
@jdantonio jdantonio merged commit c85b162 into ruby-concurrency:master Feb 24, 2015
@tenderlove
Copy link
Contributor Author

Also, this code will result in allocations of floating point objects. If we only care about resolution down to the second, we could do:

irb(main):001:0> Process.clock_gettime Process::CLOCK_MONOTONIC, :second
=> 347802
irb(main):002:0>

Which will only give integers (so they should be allocation free)

@chrisseaton
Copy link
Member

this code will result in allocations of floating point objects

Aren't small-enough Floats tagged these days?

@tenderlove
Copy link
Contributor Author

Aren't small-enough Floats tagged these days?

Actually you're totally right. If they are small enough, they won't be allocated. So nevermind! ✨

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