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

reverse.detect vs select { ... }.last #98

Open
nerdrew opened this issue Feb 11, 2016 · 5 comments
Open

reverse.detect vs select { ... }.last #98

nerdrew opened this issue Feb 11, 2016 · 5 comments

Comments

@nerdrew
Copy link

nerdrew commented Feb 11, 2016

Third approach?

require 'benchmark/ips'

ARRAY = [*1..100]

def faster
  ARRAY.reverse_each { |x| break x if (x % 10).zero? }
end

def fast
  ARRAY.reverse.detect { |x| (x % 10).zero? }
end

def slow
  ARRAY.select { |x| (x % 10).zero? }.last
end

Benchmark.ips do |x|
  x.report('Enumerable#reverse_each + break') { faster }
  x.report('Enumerable#reverse.detect') { fast }
  x.report('Enumerable#select.last')    { slow }
  x.compare!
end
% ruby reverse.rb
Calculating -------------------------------------
Enumerable#reverse_each + break
                       115.335k i/100ms
Enumerable#reverse.detect
                        72.531k i/100ms
Enumerable#select.last
                        11.243k i/100ms
-------------------------------------------------
Enumerable#reverse_each + break
                          3.311M (± 7.2%) i/s -     16.493M
Enumerable#reverse.detect
                          1.255M (± 9.3%) i/s -      6.238M
Enumerable#select.last
                        129.592k (± 3.4%) i/s -    652.094k

Comparison:
Enumerable#reverse_each + break:  3310776.9 i/s
Enumerable#reverse.detect:  1255082.9 i/s - 2.64x slower
Enumerable#select.last:   129592.0 i/s - 25.55x slower
@PeterCamilleri
Copy link

This test is biased in favor of reverse each because it bails on the first element, 100. A fairer test would be to search for (x % 51).zero?, since we expect to traverse about half the elements on average.

@nerdrew
Copy link
Author

nerdrew commented Feb 12, 2016

Results with x % 51

Comparison:
Enumerable#reverse_each + break:   257525.8 i/s
Enumerable#reverse.detect:   189938.5 i/s - 1.36x slower
Enumerable#select.last:   129624.6 i/s - 1.99x slower

@PeterCamilleri
Copy link

Wow. That sure seems to level the playing field.

@parkerfinch
Copy link
Contributor

I'd propose replacing reverse.detect with reverse_each.detect in the comparison, rather than adding it as a third option, for all the reasons listed in the reverse.each vs reverse_each section.

If there's support behind this, I'm happy to whip up a PR for it!

@corsonknowles
Copy link

Yes, reverse_each.detect and reverse_each.any? are going to be more performant than select.last or reverse.detect.
This is even documented in another benchmark on the same page. But reference material by definition usually means you look at the one relevant entry.

It's worth an update!

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

No branches or pull requests

4 participants