-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add Enumerable#last
#5844
Add Enumerable#last
#5844
Conversation
I dislike it as there can't be any efficient way to implement it. If you want to do |
Sometimes we just need to take the last element - is it more efficient to use an |
This makes no sense. Besides the fact that this can't be implemented in an efficient way, an
I don't see the significance for #5760. |
I agree with all the above. I vote to close. This already exists on |
Fine, I close this |
I would like also to post benchmarks: Benchmark.ips do |x|
x.report("last with Enumerable") do
(0..9).last
end
x.report("last with Indexable") do
(0..9).to_a[-1]
end
end Results with
Results with
Maybe I've done it wrong, but the performance difference is significant. EDIT: I've switched |
Well, creating the array in the enumerable example is unnecessary and probably adds quite a bit of overhead because of heap allocations. You should just do
|
@straight-shoota I've switched |
the correct code is to do |
The point is to avoid indexing to obtain the last value from an enumerable. Creating an array from an enumerable means indexing, and a performance penality.. |
Also, having ten numbers won't be a big difference in any case. Try with a million entries or so (and as other stated, do the allocation outside the benchmarks - those are not interesting here). |
@j8r I meant for the benchmark, not for actual usage. |
Thanks for the feedbacks: module Enumerable(T)
def last
last = uninitialized T
each { |e| last = e }
last
end
end
ENUMERABLE = (0..99999)
ARRAY = ENUMERABLE.to_a
Benchmark.ips do |x|
x.report("last with Enumerable") do
ENUMERABLE.last
end
x.report("last with Indexable") do
ARRAY.last
end
end With
Without
|
with
|
The second values can't possibly be anywhere near realistic. It should be pretty much the same as with |
I was very confused with the above benchmark results. I thought it is impossible that the results of the above benchmark on my computer:
but when running it without
In conclusion, the reason why |
looks like LLVM optimizes out the |
I have not yet seen any code for a valid benchmark with that reproducible result. The last example by @j8r is crap. It reuses the Range as iterator, which means only the first call to A proper benchmark needs to either use fresh iterator for every run or reset it in between. The following code runs in release mode without any strange optimizations and yields a plausible result: require "benchmark"
module Enumerable(T)
def last
last = uninitialized T
found = false
each { |e| last = e; found = true }
raise EmptyError.new unless found
last
end
end
ARRAY = (0..9999).to_a
Benchmark.ips do |bm|
bm.report("Enumerable#last") do
ARRAY.each.last
end
bm.report("Indexable#last") do
# ARRAY.each
ARRAY.last
end
end Of course, this is not entirely accurate, because the enumerable test also initializes a new iterator on each run (that's the 32 B heap allocation). But that's completely negligible compared to looping through thousands of array items.
To equalize this, I have added
|
But it's not crap, |
I'm pretty sure there was different code before. |
But anyway, it makes no sense to benchmark code that can leak uninitialized values. It's certainly not usable so there is no point in performance testing it. |
This PR adds
Enumerable#last
to continue the work of #5760