rubocop-performance icon indicating copy to clipboard operation
rubocop-performance copied to clipboard

Add Performance::NumericPredicate cop

Open miry opened this issue 1 year ago • 2 comments

Performance::NumericPredicate cop identifies places where numeric uses predicates like positive?, negative? and for some cases zero? should be converted to compare operator.

The Performance::NumericPredicate cop is added to identify instances where numeric predicates such as positive?, negative?, and occasionally zero? should be replaced with comparison operators for improved efficiency.

Predicates incur a performance overhead by executing a method before comparison. A small benchmark comparison between using a comparison operator (> 0) and positive? illustrates the performance difference:

x.report("compare with 0") { arr.each {|i| i > 0 } }
x.report("positive?") { arr.each {|i| i.positive? } }

Benchmark results on Ruby 3.3.0 (with YJIT) indicate a significant performance gain when using the comparison operator:

ruby 3.3.0 (2023-12-25 revision 5124f9ac75) +YJIT [arm64-darwin23]
Warming up --------------------------------------
      compare with 0     1.000 i/100ms
           positive?     1.000 i/100ms
Calculating -------------------------------------
      compare with 0      3.153 (± 0.0%) i/s -     95.000 in  30.132600s
           positive?      2.397 (± 0.0%) i/s -     72.000 in  30.042688s

Comparison:
      compare with 0:        3.2 i/s
           positive?:        2.4 i/s - 1.32x  slower

This cop is unsafe because it cannot be guaranteed that the receiver is Number and could be noisy.


Before submitting the PR make sure the following are checked:

  • [x] The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • [x] Wrote good commit messages.
  • [x] Commit message starts with [Fix #issue-number] (if the related issue exists).
  • [x] Feature branch is up-to-date with master (if not - rebase it).
  • [x] Squashed related commits together.
  • [x] Added tests.
  • [x] Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • [ ] Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

miry avatar Feb 06 '24 14:02 miry

This is just Style/NumericPredicate, is it not? It's configured for predicates by default so it would say that they will flip-flop between each other if not handled specifically. I'm also not sure if its worth the effort to duplicate a cop for what should basically be a different config value.

Earlopain avatar Sep 13 '24 14:09 Earlopain

@Earlopain Are there any established practices for handling conflicts between rubocop cops and performance cops? I wonder, if there is a way to check the RuboCop configuration to ensure that the performance-related cops are enabled.

miry avatar Sep 14 '24 18:09 miry