fastruby / fast-ruby

:dash: Writing Fast Ruby :heart_eyes: -- Collect Common Ruby idioms.
https://github.com/fastruby/fast-ruby
5.67k stars 376 forks source link

Compare Array#drop with Array#[] #181

Closed parkerfinch closed 3 years ago

parkerfinch commented 4 years ago

Endless ranges were introduced in Ruby 2.6, and are mentioned as a way to idiomatically slice arrays.

(https://ruby-doc.org/core-2.6/Range.html#class-Range-label-Endless+Ranges)

This is slower than using Enumerable#drop.

marcandre commented 3 years ago

It is not, and it shouldn't be (otherwise it would be a bug and we'd optimize MRI). Move the range creation out of your block and you will get similar results.

mblumtritt commented 3 years ago

FYI: Array#drop overrides Enumerable#drop (see array.c) , so you compare Array#[] with Array#drop

parkerfinch commented 3 years ago

It is not, and it shouldn't be (otherwise it would be a bug and we'd optimize MRI). Move the range creation out of your block and you will get similar results.

Thanks @marcandre! That makes a whole lot of sense, I hadn't considered that we're really doing two operations with the range creation and the array slicing. Just so we have the data, I confirmed that if the range construction happens elsewhere then the results of #drop and #[] are similar:

Warming up --------------------------------------
          Array#drop   824.845k i/100ms
            Array#[]   868.100k i/100ms
Array#[] and make range
                       530.367k i/100ms
Calculating -------------------------------------
          Array#drop      8.579M (± 2.3%) i/s -     42.892M in   5.002207s
            Array#[]      8.754M (± 1.9%) i/s -     44.273M in   5.059723s
Array#[] and make range
                          5.545M (± 1.3%) i/s -     28.109M in   5.069981s

Comparison:
            Array#[]:  8753503.1 i/s
          Array#drop:  8579448.9 i/s - same-ish: difference falls within error
Array#[] and make range:  5545248.3 i/s - 1.58x  (± 0.00) slower

I'm still not sure how to go about this one. The pattern I've seen is to construct the range in the #[] call, which is slower. So it seems like that pattern is still doing unnecessary work, which makes me think that the cop that was added is still reasonable.

In any case, I agree that as it stands this is not really a comparison of #drop with #[], so I'll close this out. Happy to discuss other ideas on how to think about this one, but I think we should do them in a new PR that gets the comparison right from the get-go.


FYI: Array#drop overrides Enumerable#drop (see array.c) , so you compare Array#[] with Array#drop…

Ah yes, very true, thanks @mblumtritt! I'll rename this PR before closing to reflect that.