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 monotonic clock for timestamp deltas when available #161

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

djudd
Copy link
Contributor

@djudd djudd commented Jul 6, 2021

Check for _POSIX_MONOTONIC_CLOCK and, if present, use clock_gettime with CLOCK_MONOTONIC instead of using gettimeofday.

I've verified that tests pass and use the monotonic clock on both Ubuntu 20.04.2 and macOS 11.4 (which apparently grew support for clock_gettime recently). Haven't tested on any other systems but did verify the tests still pass and don't use the monotonic clock if I munge the check.

I don't write much C, so please tell me what terrible things I've done and how to make them better.

int delta_to_first_unrecorded_gc_sample = 0;
int i;
int64_t delta_to_first_unrecorded_gc_sample = 0;
size_t i;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an incidental change but made a compiler warning go away.

@djudd
Copy link
Contributor Author

djudd commented Jul 23, 2021

Since I'm not sure @tmm1 is actively maintaining this anymore, @tenderlove want to take a look?

@tenderlove
Copy link
Collaborator

It seems fine but I'll check more seriously (not from my phone lol) tomorrow. Thanks for sending this!! It makes sense to use the monotonic clock

@djudd
Copy link
Contributor Author

djudd commented Aug 1, 2021

@tenderlove did you get a chance to check more seriously?

@tenderlove
Copy link
Collaborator

@djudd sorry, I totally forgot. Just reviewed now, and it looks great!

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