aboutsummaryrefslogtreecommitdiffstats
path: root/activesupport
diff options
context:
space:
mode:
authorbogdanvlviv <bogdanvlviv@gmail.com>2018-06-15 09:07:18 +0300
committerbogdanvlviv <bogdanvlviv@gmail.com>2018-08-14 19:53:12 +0300
commitb71abb3bb8cd177d1d3fceec88f54b505d616887 (patch)
treeedb47a67fd44ab9a6fad7a38239094f2146fdad9 /activesupport
parente765bff13484b3c834f0e9979550196898dc7cc9 (diff)
downloadrails-b71abb3bb8cd177d1d3fceec88f54b505d616887.tar.gz
rails-b71abb3bb8cd177d1d3fceec88f54b505d616887.tar.bz2
rails-b71abb3bb8cd177d1d3fceec88f54b505d616887.zip
Refactor `Array#extract!`
Avoid allocating the second array by using `Array#reject!` instead of `Enumerable#partition` in `Array#extract!`. There are benchmarks in order to ensure that the changes speed up the method: ``` begin require "bundler/inline" rescue LoadError => e $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler" raise e end class Array def extract_v1!(&block) unless block_given? to_enum(:extract!) { size } else extracted_elements, other_elements = partition(&block) replace(other_elements) extracted_elements end end def extract_v2! return to_enum(:extract!) { size } unless block_given? extracted_elements = [] reject! do |element| extracted_elements << element if yield(element) end extracted_elements end end gemfile(true) do source "https://rubygems.org" gem "benchmark-ips" end arrays_for_partition = Array.new(1000) { (0..10000).to_a } arrays_for_extract_v1 = Array.new(1000) { (0..10000).to_a } arrays_for_extract_v2 = Array.new(1000) { (0..10000).to_a } Benchmark.ips do |x| x.report("Array#partition") do arrays_for_partition.each do |numbers| odd_numbers, numbers = numbers.partition { |number| number.odd? } numbers end end x.report("Array#extract_v1!") do arrays_for_extract_v1.each do |numbers| odd_numbers = numbers.extract_v1! { |number| number.odd? } numbers end end x.report("Array#extract_v2!") do arrays_for_extract_v2.each do |numbers| odd_numbers = numbers.extract_v2! { |number| number.odd? } numbers end end x.compare! end ``` The result of the benchmarks: ``` ruby -v ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux] ``` ``` Fetching gem metadata from https://rubygems.org/. Resolving dependencies... Using benchmark-ips 2.7.2 Using bundler 1.16.1 Warming up -------------------------------------- Array#partition 1.000 i/100ms Array#extract_v1! 1.000 i/100ms Array#extract_v2! 1.000 i/100ms Calculating ------------------------------------- Array#partition 1.390 (± 0.0%) i/s - 7.000 in 5.044843s Array#extract_v1! 2.781 (± 0.0%) i/s - 14.000 in 5.050589s Array#extract_v2! 3.151 (± 0.0%) i/s - 16.000 in 5.080608s Comparison: Array#extract_v2!: 3.2 i/s Array#extract_v1!: 2.8 i/s - 1.13x slower Array#partition: 1.4 i/s - 2.27x slower ``` Avoid `unless`/`else` in favour of an early return. The double-negative of that `else` can be confusing, even though the code layout is nearly the same. Also using of early return would improve `git diff` if we needed to change this method.
Diffstat (limited to 'activesupport')
-rw-r--r--activesupport/lib/active_support/core_ext/array/extract.rb14
1 files changed, 7 insertions, 7 deletions
diff --git a/activesupport/lib/active_support/core_ext/array/extract.rb b/activesupport/lib/active_support/core_ext/array/extract.rb
index 7641cb5064..cc5a8a3f88 100644
--- a/activesupport/lib/active_support/core_ext/array/extract.rb
+++ b/activesupport/lib/active_support/core_ext/array/extract.rb
@@ -7,15 +7,15 @@ class Array
# numbers = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
# odd_numbers = numbers.extract! { |number| number.odd? } # => [1, 3, 5, 7, 9]
# numbers # => [0, 2, 4, 6, 8]
- def extract!(&block)
- unless block_given?
- to_enum(:extract!) { size }
- else
- extracted_elements, other_elements = partition(&block)
+ def extract!
+ return to_enum(:extract!) { size } unless block_given?
- replace(other_elements)
+ extracted_elements = []
- extracted_elements
+ reject! do |element|
+ extracted_elements << element if yield(element)
end
+
+ extracted_elements
end
end