-
-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lint/Loop
is recommending bad performance
#362
Comments
Lint/Loop
is recommending bad performance
On my machine without YJIT:
With YJIT enabled:
|
The performance difference exists all the way back to Ruby 2.7 (the oldest version supported by rubocop-performance):
|
Can you please post results with 3.3.0-dev? I'm seeing the performance gap is much smaller than it is in 3.2.2. I'm curious if you see the same. I don't think this should be a recommendation now that Ruby has several JIT compilers. This is the sort of thing a JIT can and should optimize. JIT compilers work best with idiomatic code; it's really hard for them to optimize code written in a clever way to exploit implementation details of the particular Ruby interpreter. E.g., on TruffleRuby there is no performance difference between the two. If a |
Certainly much narrower in Ruby 3.3:
YJIT:
|
Here’s the benchmark running on Ruby 3.3.0 without yjit, showing that
|
Is your feature request related to a problem? Please describe.
Lint/Loop recommends what we use a
loop
block instead ofwhile
oruntil
. But looking at the performance of each, I think the recommendation should be the other way around.After reading https://mastodon.social/@noteflakes/110652154677203990 I used the gist https://gist.github.com/noteflakes/d48bb74737577d1e7e6ab3954270325a to measure that
loop
is between 1.5 and 2.0 times slower thanwhile
oruntil
(on my machine). The Mastodon post mentions that with yjit, the performance difference is even larger (up to 9x).Describe the solution you'd like
Perhaps the
Lint/Loop
cop should be retired, and aPerformance/Loop
cop should be introduced instead, recommending that people don’t useloop
whenwhile
oruntil
can be used instead.The text was updated successfully, but these errors were encountered: