From da3113e8806b1319dcca8309685e191753cf6233 Mon Sep 17 00:00:00 2001 From: Holger Just Date: Tue, 23 May 2017 12:55:27 +0200 Subject: [PATCH] Avoid Array instances when normalizing Arrays / Hashes `each_with_object` allocates an array for each kv pair. Switching to the slightly more verbose but less allocatey `each_pair` eliminates array allocations. This follows the similar change in Rails: https://github.com/rails/rails/commit/960de47f0eef79d234eb3cfc47fabb470fef1529 --- lib/rackstash/fields/abstract_collection.rb | 10 ++++++---- spec/rackstash/fields/abstract_collection_spec.rb | 10 ++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/rackstash/fields/abstract_collection.rb b/lib/rackstash/fields/abstract_collection.rb index f897f80..66d2d3e 100644 --- a/lib/rackstash/fields/abstract_collection.rb +++ b/lib/rackstash/fields/abstract_collection.rb @@ -114,8 +114,9 @@ module Rackstash when Rackstash::Fields::Hash, Rackstash::Fields::Array return wrap ? value : value.raw when ::Hash - hash = value.each_with_object(Concurrent::Hash.new) do |(k, v), memo| - memo[utf8_encode(k)] = normalize(v, scope: scope) + hash = Concurrent::Hash.new + value.each_pair do |k, v| + hash[utf8_encode(k)] = normalize(v, scope: scope) end if wrap hash = Rackstash::Fields::Hash.new.tap do |hash_field| @@ -124,8 +125,9 @@ module Rackstash end return hash when ::Array, ::Set, ::Enumerator - array = value.each_with_object(Concurrent::Array.new) do |e, memo| - memo << normalize(e, scope: scope) + array = Concurrent::Array.new + value.each do |e| + array << normalize(e, scope: scope) end if wrap array = Rackstash::Fields::Array.new.tap do |array_field| diff --git a/spec/rackstash/fields/abstract_collection_spec.rb b/spec/rackstash/fields/abstract_collection_spec.rb index b62060f..c3cb104 100644 --- a/spec/rackstash/fields/abstract_collection_spec.rb +++ b/spec/rackstash/fields/abstract_collection_spec.rb @@ -224,6 +224,11 @@ describe Rackstash::Fields::AbstractCollection do expect(normalize(hash, wrap: false).keys).to all be_frozen end + it 'returns a Concurrent::Hash with wrap: false' do + hash = { 'one' => 1 } + expect(normalize(hash, wrap: false)).to be_an_instance_of(Concurrent::Hash) + end + it 'normalizes all values' do hash = { 'key' => :beepboop } @@ -288,6 +293,11 @@ describe Rackstash::Fields::AbstractCollection do expect(normalize(array, wrap: false)).to all be_frozen end + it 'returns a Concurrent::Array with wrap: false' do + array = [1, :two, 'three'] + expect(normalize(array, wrap: false)).to be_an_instance_of(Concurrent::Array) + end + it 'normalizes all values' do array = ['boop', :beep]