Refactor Process.clock_gettime uses
#43502
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Unless you are on a Ruby older than 2.3 (or some very old JRuby) `Concurrent.monotonic_time` is just `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. So might as well skip the extra method call, and more importantly it allows in many cases to pass the scale as second argument and save some floating point multiplication.
casperisfine
force-pushed
the
rework-process-clock-gettime-uses
branch
from
87f3076
to
81d0dc9
Compare
koic
added a commit
to koic/rubocop-performance
that referenced
this pull request
Oct 30, 2021
Follow up to rails/rails#43502. This PR adds new `Performance/ConcurrentMonotonicTime` cop. This cop identifies places where `Concurrent.monotonic_time` can be replaced by `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. ```ruby # bad Concurrent.monotonic_time # good Process.clock_gettime(Process::CLOCK_MONOTONIC) ``` Below is a benchmark. ```console % ruby monotonic_time.rb "ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-darwin19]\n" Warming up -------------------------------------- Concurrent.monotonic_time 607.064k i/100ms Process.clock_gettime 730.862k i/100ms Calculating ------------------------------------- Concurrent.monotonic_time 5.695M (± 4.4%) i/s - 28.532M in 5.019798s Process.clock_gettime 7.398M (± 2.5%) i/s - 37.274M in 5.041776s Comparison: Process.clock_gettime: 7397700.9 i/s Concurrent.monotonic_time: 5695385.0 i/s - 1.30x (± 0.00) slower ``` And this cop is compatible with ruby-concurrency/concurrent-ruby#915.
koic
added a commit
to koic/rubocop-performance
that referenced
this pull request
Oct 30, 2021
Follow up to rails/rails#43502. This PR adds new `Performance/ConcurrentMonotonicTime` cop. This cop identifies places where `Concurrent.monotonic_time` can be replaced by `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. ```ruby # bad Concurrent.monotonic_time # good Process.clock_gettime(Process::CLOCK_MONOTONIC) ``` Below is a benchmark. ```console % ruby monotonic_time.rb "ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-darwin19]\n" Warming up -------------------------------------- Concurrent.monotonic_time 607.064k i/100ms Process.clock_gettime 730.862k i/100ms Calculating ------------------------------------- Concurrent.monotonic_time 5.695M (± 4.4%) i/s - 28.532M in 5.019798s Process.clock_gettime 7.398M (± 2.5%) i/s - 37.274M in 5.041776s Comparison: Process.clock_gettime: 7397700.9 i/s Concurrent.monotonic_time: 5695385.0 i/s - 1.30x (± 0.00) slower ``` And this cop is compatible with ruby-concurrency/concurrent-ruby#915.
8 tasks
koic
added a commit
to koic/rubocop-performance
that referenced
this pull request
Oct 30, 2021
Follow up to rails/rails#43502. This PR adds new `Performance/ConcurrentMonotonicTime` cop. This cop identifies places where `Concurrent.monotonic_time` can be replaced by `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. ```ruby # bad Concurrent.monotonic_time # good Process.clock_gettime(Process::CLOCK_MONOTONIC) ``` Below is a benchmark. ```ruby % cat monotonic_time.rb #!/usr/local/bin/ruby p `ruby -v` require 'concurrent' require 'benchmark/ips' Benchmark.ips do |x| x.report('Concurrent.monotonic_time') { Concurrent.monotonic_time } x.report('Process.clock_gettime') { Process.clock_gettime(Process::CLOCK_MONOTONIC) } x.compare! end ``` ```console % ruby monotonic_time.rb "ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-darwin19]\n" Warming up -------------------------------------- Concurrent.monotonic_time 607.064k i/100ms Process.clock_gettime 730.862k i/100ms Calculating ------------------------------------- Concurrent.monotonic_time 5.695M (± 4.4%) i/s - 28.532M in 5.019798s Process.clock_gettime 7.398M (± 2.5%) i/s - 37.274M in 5.041776s Comparison: Process.clock_gettime: 7397700.9 i/s Concurrent.monotonic_time: 5695385.0 i/s - 1.30x (± 0.00) slower ``` And this cop is compatible with ruby-concurrency/concurrent-ruby#915.
eregon
reviewed
Nov 22, 2021
Comment on lines
+161
to
+163
| begin | ||
| GC.stat(:total_allocated_objects) | ||
| rescue ArgumentError # Likely on JRuby |
There was a problem hiding this comment.
Thanks for this change, TruffleRuby also cannot report the number of allocated objects (not exposed on JVM) so this might be helpful :)
7 tasks
patrickm53
pushed a commit
to patrickm53/performance-develop-rubyonrails
that referenced
this pull request
Sep 23, 2022
Follow up to rails/rails#43502. This PR adds new `Performance/ConcurrentMonotonicTime` cop. This cop identifies places where `Concurrent.monotonic_time` can be replaced by `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. ```ruby # bad Concurrent.monotonic_time # good Process.clock_gettime(Process::CLOCK_MONOTONIC) ``` Below is a benchmark. ```ruby % cat monotonic_time.rb #!/usr/local/bin/ruby p `ruby -v` require 'concurrent' require 'benchmark/ips' Benchmark.ips do |x| x.report('Concurrent.monotonic_time') { Concurrent.monotonic_time } x.report('Process.clock_gettime') { Process.clock_gettime(Process::CLOCK_MONOTONIC) } x.compare! end ``` ```console % ruby monotonic_time.rb "ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-darwin19]\n" Warming up -------------------------------------- Concurrent.monotonic_time 607.064k i/100ms Process.clock_gettime 730.862k i/100ms Calculating ------------------------------------- Concurrent.monotonic_time 5.695M (± 4.4%) i/s - 28.532M in 5.019798s Process.clock_gettime 7.398M (± 2.5%) i/s - 37.274M in 5.041776s Comparison: Process.clock_gettime: 7397700.9 i/s Concurrent.monotonic_time: 5695385.0 i/s - 1.30x (± 0.00) slower ``` And this cop is compatible with ruby-concurrency/concurrent-ruby#915.
richardstewart0213
added a commit
to richardstewart0213/performance-build-Performance-optimization-analysis-
that referenced
this pull request
Nov 4, 2022
Follow up to rails/rails#43502. This PR adds new `Performance/ConcurrentMonotonicTime` cop. This cop identifies places where `Concurrent.monotonic_time` can be replaced by `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. ```ruby # bad Concurrent.monotonic_time # good Process.clock_gettime(Process::CLOCK_MONOTONIC) ``` Below is a benchmark. ```ruby % cat monotonic_time.rb #!/usr/local/bin/ruby p `ruby -v` require 'concurrent' require 'benchmark/ips' Benchmark.ips do |x| x.report('Concurrent.monotonic_time') { Concurrent.monotonic_time } x.report('Process.clock_gettime') { Process.clock_gettime(Process::CLOCK_MONOTONIC) } x.compare! end ``` ```console % ruby monotonic_time.rb "ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-darwin19]\n" Warming up -------------------------------------- Concurrent.monotonic_time 607.064k i/100ms Process.clock_gettime 730.862k i/100ms Calculating ------------------------------------- Concurrent.monotonic_time 5.695M (± 4.4%) i/s - 28.532M in 5.019798s Process.clock_gettime 7.398M (± 2.5%) i/s - 37.274M in 5.041776s Comparison: Process.clock_gettime: 7397700.9 i/s Concurrent.monotonic_time: 5695385.0 i/s - 1.30x (± 0.00) slower ``` And this cop is compatible with ruby-concurrency/concurrent-ruby#915.
Cute0110
added a commit
to Cute0110/Rubocop-Performance
that referenced
this pull request
Sep 28, 2023
Follow up to rails/rails#43502. This PR adds new `Performance/ConcurrentMonotonicTime` cop. This cop identifies places where `Concurrent.monotonic_time` can be replaced by `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. ```ruby # bad Concurrent.monotonic_time # good Process.clock_gettime(Process::CLOCK_MONOTONIC) ``` Below is a benchmark. ```ruby % cat monotonic_time.rb #!/usr/local/bin/ruby p `ruby -v` require 'concurrent' require 'benchmark/ips' Benchmark.ips do |x| x.report('Concurrent.monotonic_time') { Concurrent.monotonic_time } x.report('Process.clock_gettime') { Process.clock_gettime(Process::CLOCK_MONOTONIC) } x.compare! end ``` ```console % ruby monotonic_time.rb "ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-darwin19]\n" Warming up -------------------------------------- Concurrent.monotonic_time 607.064k i/100ms Process.clock_gettime 730.862k i/100ms Calculating ------------------------------------- Concurrent.monotonic_time 5.695M (± 4.4%) i/s - 28.532M in 5.019798s Process.clock_gettime 7.398M (± 2.5%) i/s - 37.274M in 5.041776s Comparison: Process.clock_gettime: 7397700.9 i/s Concurrent.monotonic_time: 5695385.0 i/s - 1.30x (± 0.00) slower ``` And this cop is compatible with ruby-concurrency/concurrent-ruby#915.
SerhiiMisiura
added a commit
to SerhiiMisiura/Rubocop-Performance
that referenced
this pull request
Oct 5, 2023
Follow up to rails/rails#43502. This PR adds new `Performance/ConcurrentMonotonicTime` cop. This cop identifies places where `Concurrent.monotonic_time` can be replaced by `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. ```ruby # bad Concurrent.monotonic_time # good Process.clock_gettime(Process::CLOCK_MONOTONIC) ``` Below is a benchmark. ```ruby % cat monotonic_time.rb #!/usr/local/bin/ruby p `ruby -v` require 'concurrent' require 'benchmark/ips' Benchmark.ips do |x| x.report('Concurrent.monotonic_time') { Concurrent.monotonic_time } x.report('Process.clock_gettime') { Process.clock_gettime(Process::CLOCK_MONOTONIC) } x.compare! end ``` ```console % ruby monotonic_time.rb "ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-darwin19]\n" Warming up -------------------------------------- Concurrent.monotonic_time 607.064k i/100ms Process.clock_gettime 730.862k i/100ms Calculating ------------------------------------- Concurrent.monotonic_time 5.695M (± 4.4%) i/s - 28.532M in 5.019798s Process.clock_gettime 7.398M (± 2.5%) i/s - 37.274M in 5.041776s Comparison: Process.clock_gettime: 7397700.9 i/s Concurrent.monotonic_time: 5695385.0 i/s - 1.30x (± 0.00) slower ``` And this cop is compatible with ruby-concurrency/concurrent-ruby#915.
casperisfine
pushed a commit
to Shopify/rails
that referenced
this pull request
Jan 17, 2024
Ref: rails#43502 Fix: rails#50767 Fix: rails#50493 When republishing a an event into a `start, finish` tuple, we need to convert the timestamps back into seconds.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref: ruby-concurrency/concurrent-ruby#915
Two things here
Concurrent.monotonic_timeThis was useful a long time ago (pre Ruby 2.3), but now all the supported rubies have
Process.clock_gettime(Process::CLOCK_MONOTONIC), so we might as well save a method call, especially in these tight loops.unitargument.Rather than applying
* 1000to get milliseconds, you can directly ask for milliseconds withProcess.clock_gettime(Process::CLOCK_MONOTONIC, :float_millisecond).Opening a PR simply to make sure I didn't break CI.