Skip to content
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

Fixes the 'planets align' condition of select.last vs reverse.detect #122

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mark-d-holmberg
Copy link

They're getting a false positive because it's not actually doing any work. 100 just happens
to be at the last of the list returned by .select, so it's not causing the algorithm to actually
do any work. When you change the conditions to look for a result that is modulable by 60, or to
look for the 1 element (which would literally be last), then we see different results. The test
is skewed.

Ruby 2.2.3p173


Looking for 60

ruby -v code/enumerable/select-last-vs-reverse-detect.rb

ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-linux]
Calculating -------------------------------------
Enumerable#reverse.detect
12.595k i/100ms
Enumerable#select.last
16.488k i/100ms

Enumerable#reverse.detect
127.522k (± 7.3%) i/s - 642.345k
Enumerable#select.last
178.716k (± 3.7%) i/s - 906.840k

Comparison:
Enumerable#select.last: 178715.9 i/s
Enumerable#reverse.detect: 127521.8 i/s - 1.40x slower

Looking for the '1' element

ruby -v code/enumerable/select-last-vs-reverse-detect.rb

ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-linux]
Calculating -------------------------------------
Enumerable#reverse.detect
15.121k i/100ms
Enumerable#select.last
21.968k i/100ms

Enumerable#reverse.detect
160.443k (± 1.0%) i/s - 816.534k
Enumerable#select.last
241.965k (± 1.4%) i/s - 1.230M

Comparison:
Enumerable#select.last: 241965.1 i/s
Enumerable#reverse.detect: 160442.8 i/s - 1.51x slower

Normal Operation

ruby -v code/enumerable/select-last-vs-reverse-detect.rb

ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-linux]
Calculating -------------------------------------
Enumerable#reverse.detect
99.839k i/100ms
Enumerable#select.last
14.585k i/100ms

Enumerable#reverse.detect
1.717M (± 1.7%) i/s - 8.586M
Enumerable#select.last
158.330k (± 2.9%) i/s - 802.175k

Comparison:
Enumerable#reverse.detect: 1716784.6 i/s
Enumerable#select.last: 158329.7 i/s - 10.84x slower

Ruby 2.4.1p111


Looking for 60

ruby -v code/enumerable/select-last-vs-reverse-detect.rb
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
Warming up --------------------------------------
Enumerable#reverse.detect
15.440k i/100ms
Enumerable#select.last
18.730k i/100ms
Calculating -------------------------------------
Enumerable#reverse.detect
160.408k (± 1.8%) i/s - 802.880k in 5.006961s
Enumerable#select.last
197.193k (± 1.5%) i/s - 992.690k in 5.035280s

Comparison:
Enumerable#select.last: 197192.5 i/s
Enumerable#reverse.detect: 160407.8 i/s - 1.23x slower

Looking for the '1' element

ruby -v code/enumerable/select-last-vs-reverse-detect.rb
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
Warming up --------------------------------------
Enumerable#reverse.detect
18.808k i/100ms
Enumerable#select.last
24.390k i/100ms
Calculating -------------------------------------
Enumerable#reverse.detect
198.816k (± 2.8%) i/s - 996.824k in 5.018123s
Enumerable#select.last
261.380k (± 2.4%) i/s - 1.317M in 5.041952s

Comparison:
Enumerable#select.last: 261380.0 i/s
Enumerable#reverse.detect: 198815.7 i/s - 1.31x slower

Normal Operation

ruby -v code/enumerable/select-last-vs-reverse-detect.rb
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
Warming up --------------------------------------
Enumerable#reverse.detect
143.116k i/100ms
Enumerable#select.last
16.307k i/100ms
Calculating -------------------------------------
Enumerable#reverse.detect
1.937M (± 1.3%) i/s - 9.732M in 5.026300s
Enumerable#select.last
167.070k (± 7.1%) i/s - 847.964k in 5.121300s

Comparison:
Enumerable#reverse.detect: 1936536.3 i/s
Enumerable#select.last: 167070.4 i/s - 11.59x slower

Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mark-d-holmberg Thanks for submitting this! How do you suggest we fix this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants