rubocop / rubocop-performance

An extension of RuboCop focused on code performance checks.
https://docs.rubocop.org/rubocop-performance
MIT License
671 stars 79 forks source link

Make `Performance/RedundantMerge` aware of `Hash#update` (or create similar cop) #393

Open ydakuka opened 9 months ago

ydakuka commented 9 months ago

Describe the solution you'd like

This rule is implemented by the cop. However, the following rule is not.

Rubocop

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop -V
1.57.2 (using Parser 3.2.2.4, rubocop-ast 1.29.0, running on ruby 2.7.8) [x86_64-linux]
  - rubocop-capybara 2.19.0
  - rubocop-factory_bot 2.24.0
  - rubocop-performance 1.19.1
  - rubocop-rails 2.22.0
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.25.0
  - rubocop-thread_safety 0.5.1
ydakuka commented 9 months ago

I've benchmarked it (ruby 3.3.0).

1.

Hash#merge! vs Hash#[]=

Test:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
         Hash#merge!     3.485k i/100ms
            Hash#[]=     8.717k i/100ms
Calculating -------------------------------------
         Hash#merge!     34.430k (± 1.0%) i/s -    174.250k in   5.061500s
            Hash#[]=     84.526k (± 1.1%) i/s -    427.133k in   5.053909s

Comparison:
            Hash#[]=:    84525.9 i/s
         Hash#merge!:    34430.1 i/s - 2.46x  slower

2.

Hash#update vs Hash#[]=

Test:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
            Hash#[]=     7.989k i/100ms
         Hash#update     3.193k i/100ms
Calculating -------------------------------------
            Hash#[]=     78.912k (± 1.3%) i/s -    399.450k in   5.062829s
         Hash#update     31.624k (± 1.6%) i/s -    159.650k in   5.049574s

Comparison:
            Hash#[]=:    78911.9 i/s
         Hash#update:    31624.2 i/s - 2.50x  slower
gstokkink commented 2 months ago

@koic seems like a no-brainer? 😄