From b0cae1814a07f34eb014d1c1e9f739a846c76d3c Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 30 Oct 2021 11:24:01 +0900 Subject: [PATCH] Add new `Performance/ConcurrentMonotonicTime` cop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow up to https://github.com/rails/rails/pull/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 https://github.com/ruby-concurrency/concurrent-ruby/pull/915. --- ...rformance_concurrent_monotonic_time_cop.md | 1 + config/default.yml | 6 +++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_performance.adoc | 30 +++++++++++++ .../performance/concurrent_monotonic_time.rb | 42 +++++++++++++++++++ lib/rubocop/cop/performance_cops.rb | 1 + .../concurrent_monotonic_time_spec.rb | 42 +++++++++++++++++++ 7 files changed, 123 insertions(+) create mode 100644 changelog/new_add_new_performance_concurrent_monotonic_time_cop.md create mode 100644 lib/rubocop/cop/performance/concurrent_monotonic_time.rb create mode 100644 spec/rubocop/cop/performance/concurrent_monotonic_time_spec.rb diff --git a/changelog/new_add_new_performance_concurrent_monotonic_time_cop.md b/changelog/new_add_new_performance_concurrent_monotonic_time_cop.md new file mode 100644 index 0000000000..d4b6e10da3 --- /dev/null +++ b/changelog/new_add_new_performance_concurrent_monotonic_time_cop.md @@ -0,0 +1 @@ +* [#267](https://github.com/rubocop/rubocop-performance/pull/267): Add new `Performance/ConcurrentMonotonicTime` cop. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index 9decfdd4ac..ac8241eca3 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index b11207f7ae..5f2ced45dd 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -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] diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index a6b5236e4b..f824b5614d 100644 --- a/docs/modules/ROOT/pages/cops_performance.adoc +++ b/docs/modules/ROOT/pages/cops_performance.adoc @@ -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 |=== diff --git a/lib/rubocop/cop/performance/concurrent_monotonic_time.rb b/lib/rubocop/cop/performance/concurrent_monotonic_time.rb new file mode 100644 index 0000000000..a636b00cfb --- /dev/null +++ b/lib/rubocop/cop/performance/concurrent_monotonic_time.rb @@ -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 `%s` instead of `%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 diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index 53f8a9330d..fc363b712d 100644 --- a/lib/rubocop/cop/performance_cops.rb +++ b/lib/rubocop/cop/performance_cops.rb @@ -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' diff --git a/spec/rubocop/cop/performance/concurrent_monotonic_time_spec.rb b/spec/rubocop/cop/performance/concurrent_monotonic_time_spec.rb new file mode 100644 index 0000000000..d1022c7b14 --- /dev/null +++ b/spec/rubocop/cop/performance/concurrent_monotonic_time_spec.rb @@ -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