Skip to content

Commit

Permalink
Optimize Base.callbacks_needed
Browse files Browse the repository at this point in the history
This matters since there are quite a few public methods per cop class.
In total, it gets called ~57k times, or ~104 times per cop class.

`method_defined?` is faster than string checking, so move that first. It also eliminates most of the checks, Base contains 98 methods.

Current:
```
$ hyperfine -w 10 -r 25 "bundle exec rubocop Rakefile --cache=false"
Benchmark 1: bundle exec rubocop Rakefile --cache=false
  Time (mean ± σ):     932.8 ms ±   7.6 ms    [User: 828.0 ms, System: 104.0 ms]
  Range (min … max):   919.5 ms … 952.2 ms    25 runs
```

With String:
```
$ hyperfine -w 10 -r 25 "bundle exec rubocop Rakefile --cache=false"
Benchmark 1: bundle exec rubocop Rakefile --cache=false
  Time (mean ± σ):     924.1 ms ±   7.8 ms    [User: 816.3 ms, System: 107.3 ms]
  Range (min … max):   912.8 ms … 941.5 ms    25 runs
```

With String + order switched:
```
$ hyperfine -w 10 -r 25 "bundle exec rubocop Rakefile --cache=false"
Benchmark 1: bundle exec rubocop Rakefile --cache=false
  Time (mean ± σ):     920.7 ms ±   6.9 ms    [User: 814.5 ms, System: 105.6 ms]
  Range (min … max):   906.9 ms … 932.5 ms    25 runs
```

~ 1.3% faster
  • Loading branch information
Earlopain committed Sep 18, 2024
1 parent 8fc2afc commit ed1aadb
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/rubocop/cop/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,12 @@ def callbacks_needed
# @api private
def self.callbacks_needed
@callbacks_needed ||= public_instance_methods.select do |m|
m.start_with?(/on_|after_/) &&
!Base.method_defined?(m) # exclude standard "callbacks" like 'on_begin_investigation'
# OPTIMIZE: Check method existence first to make fewer `start_with?` calls.
# At the time of writing this comment, this excludes 98 of ~104 methods.
# `start_with?` with two string arguments instead of a regex is faster
# in this specific case as well.
!Base.method_defined?(m) && # exclude standard "callbacks" like 'on_begin_investigation'
m.start_with?('on_', 'after_')
end
end
# rubocop:enable Layout/ClassStructure
Expand Down

0 comments on commit ed1aadb

Please sign in to comment.