rubocop / rubocop-performance

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

Add Performance::NumericPredicate cop #440

Open miry opened 8 months ago

miry commented 8 months ago

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:

Earlopain commented 1 month ago

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.

miry commented 1 month ago

@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.