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.

Sign up for GitHub

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

Jump to bottom

(PUP-7520) Update Puppet profiling timers to use monotonic clock #9418

Merged
merged 1 commit into from
Jul 22, 2024
Merged

(PUP-7520) Update Puppet profiling timers to use monotonic clock #9418

merged 1 commit into from
Jul 22, 2024

Conversation

AriaXLi
Copy link
Contributor

@AriaXLi AriaXLi commented Jul 17, 2024

No description provided.

@AriaXLi AriaXLi requested a review from a team as a code owner July 17, 2024 21:05
@AriaXLi AriaXLi force-pushed the PUP-7520 branch 2 times, most recently from 2b69ffb to 2544f87 Compare July 18, 2024 18:56
@AriaXLi
(PUP-7520) Update Puppet profiling timers to use monotonic clock
e0d1778 
This commit updates Puppet::Util::Profiler::WallClock and
Puppet::Util::Profiler::Aggregate to use
Process.clock_gettime(Process::CLOCK_MONOTONIC) instead of Time.now. Time.now
returns a value from the OS REALTIME clock which can be affected by events
like NTP updates. Process.clock_gettime(Process::CLOCK_MONOTONIC) uses a
monotonic clock which isn't affected by things like NTP updates.
@AriaXLi
Copy link
Contributor Author

AriaXLi commented Jul 18, 2024

Although the ticket calls for it, I chose to not update the Puppet::Util.thinmark method since it uses Benchmark.realtime() which already uses a monotonic clock to calculate the elapsed time (see here).

@AriaXLi
Copy link
Contributor Author

AriaXLi commented Jul 18, 2024 •
edited
Loading

I also confirmed that Process.clock_gettime(Process::CLOCK_MONOTONIC) returns seconds including decimal micro/nano seconds so that we don’t change the time scale of our profile.

irb(main):007> Time.now
=> 2024-07-18 12:20:00.839343 -0700
irb(main):008> start = Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second)
=> 617210.278262
irb(main):009> Time.now
=> 2024-07-18 12:20:12.204282 -0700
irb(main):010> Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) - start
=> 11.99554800009355

@joshcooper joshcooper merged commit 82ad86e into puppetlabs:main Jul 22, 2024
9 checks passed
@joshcooper joshcooper added the bug Something isn't working label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshcooper joshcooper joshcooper approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AriaXLi @joshcooper