Skip to content

Commit

Permalink
Add new Performance/ConcurrentMonotonicTime cop
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Cute0110 committed Oct 30, 2021
1 parent e57e74d commit c04299b
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#267](https://github.com/rubocop/rubocop-performance/pull/267): Add new `Performance/ConcurrentMonotonicTime` cop. ([@koic][])
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ Performance/CompareWithBlock:
Enabled: true
VersionAdded: '0.46'

Performance/ConcurrentMonotonicTime:
Description: 'Use `Process.clock_gettime(Process::CLOCK_MONOTONIC)` instead of `Concurrent.monotonic_time`.'
Reference: 'https://github.com/rails/rails/pull/43502'
Enabled: pending
VersionAdded: '1.12'

Performance/ConstantRegexp:
Description: 'Finds regular expressions with dynamic components that are all constants.'
Enabled: pending
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Performance cops optimization analysis for your projects.
* xref:cops_performance.adoc#performancechainarrayallocation[Performance/ChainArrayAllocation]
* xref:cops_performance.adoc#performancecollectionliteralinloop[Performance/CollectionLiteralInLoop]
* xref:cops_performance.adoc#performancecomparewithblock[Performance/CompareWithBlock]
* xref:cops_performance.adoc#performanceconcurrentmonotonictime[Performance/ConcurrentMonotonicTime]
* xref:cops_performance.adoc#performanceconstantregexp[Performance/ConstantRegexp]
* xref:cops_performance.adoc#performancecount[Performance/Count]
* xref:cops_performance.adoc#performancedeleteprefix[Performance/DeletePrefix]
Expand Down
30 changes: 30 additions & 0 deletions docs/modules/ROOT/pages/cops_performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,36 @@ array.min_by(&:foo)
array.sort_by { |a| a[:foo] }
----

== Performance/ConcurrentMonotonicTime

|===
| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed

| Pending
| Yes
| Yes
| 1.12
| -
|===

This cop identifies places where `Concurrent.monotonic_time`
can be replaced by `Process.clock_gettime(Process::CLOCK_MONOTONIC)`.

=== Examples

[source,ruby]
----
# bad
Concurrent.monotonic_time
# good
Process.clock_gettime(Process::CLOCK_MONOTONIC)
----

=== References

* https://github.com/rails/rails/pull/43502

== Performance/ConstantRegexp

|===
Expand Down
42 changes: 42 additions & 0 deletions lib/rubocop/cop/performance/concurrent_monotonic_time.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Performance
# This cop identifies places where `Concurrent.monotonic_time`
# can be replaced by `Process.clock_gettime(Process::CLOCK_MONOTONIC)`.
#
# @example
#
# # bad
# Concurrent.monotonic_time
#
# # good
# Process.clock_gettime(Process::CLOCK_MONOTONIC)
#
class ConcurrentMonotonicTime < Base
extend AutoCorrector

MSG = 'Use `%<prefer>s` instead of `%<current>s`.'
RESTRICT_ON_SEND = %i[monotonic_time].freeze

def_node_matcher :concurrent_monotonic_time?, <<~PATTERN
(send
(const {nil? cbase} :Concurrent) :monotonic_time ...)
PATTERN

def on_send(node)
return unless concurrent_monotonic_time?(node)

optional_unit_parameter = ", #{node.first_argument.source}" if node.first_argument
prefer = "Process.clock_gettime(Process::CLOCK_MONOTONIC#{optional_unit_parameter})"
message = format(MSG, prefer: prefer, current: node.source)

add_offense(node, message: message) do |corrector|
corrector.replace(node, prefer)
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/performance_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
require_relative 'performance/casecmp'
require_relative 'performance/collection_literal_in_loop'
require_relative 'performance/compare_with_block'
require_relative 'performance/concurrent_monotonic_time'
require_relative 'performance/constant_regexp'
require_relative 'performance/count'
require_relative 'performance/delete_prefix'
Expand Down
42 changes: 42 additions & 0 deletions spec/rubocop/cop/performance/concurrent_monotonic_time_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Performance::ConcurrentMonotonicTime, :config do
it 'registers an offense when using `Concurrent.monotonic_time`' do
expect_offense(<<~RUBY)
Concurrent.monotonic_time
^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Process.clock_gettime(Process::CLOCK_MONOTONIC)` instead of `Concurrent.monotonic_time`.
RUBY

expect_correction(<<~RUBY)
Process.clock_gettime(Process::CLOCK_MONOTONIC)
RUBY
end

it 'registers an offense when using `::Concurrent.monotonic_time`' do
expect_offense(<<~RUBY)
::Concurrent.monotonic_time
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Process.clock_gettime(Process::CLOCK_MONOTONIC)` instead of `::Concurrent.monotonic_time`.
RUBY

expect_correction(<<~RUBY)
Process.clock_gettime(Process::CLOCK_MONOTONIC)
RUBY
end

it 'registers an offense when using `Concurrent.monotonic_time` with an optional unit parameter' do
expect_offense(<<~RUBY)
Concurrent.monotonic_time(:float_second)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second)` instead of `Concurrent.monotonic_time(:float_second)`.
RUBY

expect_correction(<<~RUBY)
Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second)
RUBY
end

it 'does not register an offense when using `Process.clock_gettime(Process::CLOCK_MONOTONIC)`' do
expect_no_offenses(<<~RUBY)
Process.clock_gettime(Process::CLOCK_MONOTONIC)
RUBY
end
end

0 comments on commit c04299b

Please sign in to comment.