Add benchmark for Array#reject vs Array#delete vs Array#delete_if vs Array-[nil] #101
Conversation
|
Definitely need to add |
|
Here you have it, @nateberkopec good catch! |
|
|
There are some problems with this:
This should be more accurate, hopefully I didn't overlook anything else: require 'benchmark/ips'
ARRAY = Array.new(100_000){ rand(2).nonzero? }
ARRAY1 = ARRAY.dup
ARRAY2 = ARRAY.dup
ARRAY3 = ARRAY.dup
ARRAY4 = ARRAY.dup
ARRAY5 = ARRAY.dup
ARRAY6 = ARRAY.dup
ARRAY7 = ARRAY.dup
ARRAY8 = ARRAY.dup
def compact!
ARRAY1.compact!
end
def compact
ARRAY2.compact
end
def reject!
ARRAY3.reject! { |o| o.nil? }
end
def reject
ARRAY4.reject { |o| o.nil? }
end
def delete
ARRAY5.delete(nil)
end
def delete_if
ARRAY6.delete_if { |o| o.nil? }
end
def subtract
ARRAY7 - [nil]
end
def grep_v
ARRAY8.grep_v(nil)
end
Benchmark.ips do |x|
x.report('A.compact!') { compact! }
x.report('A.compact') { compact }
x.report('A.reject{|o|o.nil?}') { reject }
x.report('A.reject!{|o|o.nil?}') { reject! }
x.report('A.delete(nil)') { delete }
x.report('A.delete_if{|o|o.nil?}') { delete_if }
x.report('A - [nil]') { subtract }
x.report('A.grep_v(nil)') { grep_v }
x.compare!
end |
|
|
||
|
|
||
| def reject | ||
| ARRAY.reject { |o| o.nil? } |
There was a problem hiding this comment.
@ixti looks good to me use that syntax, will take on mind when made the changes proposed.
I didn't think on the mutation of the array, that was a good lesson to keep in mind.
|
@Arcovion Thanks for your review, will do the changes, this was something I found in my daily work and this was the repo to document my founds. |
|
Results after code review: |
|
I'm thinking about Thoughts? |
|
Thanks! LGTM. |
|
After thinking a bit, I'm not sure how "good" comparison with bang versions are. |
|
Perhaps we could do two separate benchmarks in the same file - one comparing the bang/modify-in-place versions, one without? |
|
I was thinking about that too. But after first iteration |
|
@ixti Oh wow, I completely forgot that. For some reason I was thinking that only mattered between each run of |
|
How about: require 'benchmark/ips'
ARRAY = Array.new(1000){ rand(2).nonzero? }
def compact
ARRAY.compact
end
def reject
ARRAY.reject(&:nil?)
end
def delete
ARRAY.dup.delete(nil)
end
def delete_if
ARRAY.dup.delete_if(&:nil?)
end
def subtract
ARRAY - [nil]
end
def grep_v
ARRAY.grep_v(nil)
end
Benchmark.ips do |x|
x.report('A.compact') { compact }
x.report('A.reject(&:nil?)') { reject }
x.report('A.dup.delete(nil)') { delete }
x.report('A.dup.delete_if(&:nil?)') { delete_if }
x.report('A - [nil]') { subtract }
x.report('A.grep_v(nil)') { grep_v }
x.compare!
end |
|
Yeah. Looks good. |
|
Hi guys - I have two remarks:
|
|
I tend to agree that probably it worth to avoid |
require 'benchmark/ips'
ARRAY = Array.new(1000){ rand(2).nonzero? }
def compact
ARRAY.compact
end
def reject
ARRAY.reject(&:nil?)
end
def subtract
ARRAY - [nil]
end
Benchmark.ips do |x|
x.report('A.compact') { compact }
x.report('A.reject(&:nil?)') { reject }
x.report('A - [nil]') { subtract }
x.compare!
end |
require 'benchmark/ips'
ARRAY = Array.new(1000){ rand(2).nonzero? }
ARRAY1 = ARRAY.dup
ARRAY2 = ARRAY.dup
ARRAY3 = ARRAY.dup
def grep_v
ARRAY1.grep_v(nil)
end
def delete
ARRAY2.delete(nil)
end
def delete_if
ARRAY3.delete_if(&:nil?)
end
Benchmark.ips do |x|
x.report('A.grep_v(nil)') { grep_v }
x.report('A.delete(nil)') { delete }
x.report('A.delete_if(&:nil?)') { delete_if }
x.compare!
end |
|
@gbalbuena I'm sorry you've had to make so many changes for what was such a simple PR. I wondered if we could test stuff by adding What we know for sure is that I think having a benchmark for removing nils is a bit pointless to be honest, Anyone else got any thoughts or ideas? Help me out... |
|
But I think the cleanup of |
To be fair, a good portion of the benchmarks we have with Enumerable boil down to "use this C-optimized method instead of ". Which is fine - I think it's easy to reach for the Enumerable method because you use them so often, rather than specialized methods like |
|
It's also good to not rely too heavily on MRI's internals. Those assumptions naturally do not hold for other Ruby implementations and possibly might not hold for future versions of MRI. |
|
I was disappointed to see this closed without resolution, so I created a gem to help with these kind of benchmarks: benchmark-inputs. (Feedback welcome!) Here is the benchmark code comparing all cases: require 'benchmark/inputs'
NO_NILS = [1] * 100
SOME_NILS = [*1..9, nil] * 10
JUST_NIL = [nil]
Benchmark.inputs([NO_NILS, SOME_NILS]) do |x|
x.dup_inputs = true # enabled for destructive operations
x.report('A.compact') {|a| a.compact }
x.report('A.compact!') {|a| a.compact! }
x.report('A.reject(&:nil?)') {|a| a.reject(&:nil?) }
x.report('A.reject!(&:nil?)') {|a| a.reject!(&:nil?) }
x.report('A.delete(nil)') {|a| a.delete(nil) }
x.report('A.delete_if(&:nil?)') {|a| a.delete_if(&:nil?) }
x.report('A - [nil]'){|a| a - JUST_NIL }
#x.report('A.grep_v(nil)'){|a| a.grep_v(nil) } # only in Ruby 2.3+
x.compare!
endAnd here are the results I get: The standard deviation of Benchmark.ips do |x|
x.report('A.dup.compact!') { SOME_NILS.dup.compact! }
x.report('A.dup.delete(nil)') { SOME_NILS.dup.delete(nil) }
x.compare!
end |
When you need to cleanup the array and extract all the nils there are at least 4 different ways
These are the results: