From 964e7ad1566f075396b40c6912d241772269aea7 Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Wed, 12 Aug 2020 16:22:46 -0700 Subject: [PATCH] deep dup Is there a reason we're only taking this short cut for frozen strings and not other immutable objects commonly used as hash keys (eg. symbols and integers)? initial tests suggest a ~30% performance improvement... ``` require 'benchmark' class Object def deep_dup dup end alias deep_dup_all deep_dup end class Hash def deep_dup hash = dup each_pair do |key, value| if key.frozen? && ::String === key hash[key] = value.deep_dup else hash.delete(key) hash[key.deep_dup] = value.deep_dup end end hash end def deep_dup_all hash = dup each_pair do |key, value| if key.frozen? hash[key] = value.deep_dup_all else hash.delete(key) hash[key.deep_dup_all] = value.deep_dup_all end end hash end end data = Hash[('aaa'..'zzz').map {|k| [k.to_sym, k]}] control = Benchmark.realtime do 30.times { data.deep_dup } end experiment = Benchmark.realtime do 30.times { data.deep_dup_all } end puts "%.3f v %.3f => %d%% speed up" % [ control, experiment, (control - experiment) / control * 100 ] ``` --- .../lib/active_support/core_ext/object/deep_dup.rb | 2 +- activesupport/test/core_ext/object/deep_dup_test.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/core_ext/object/deep_dup.rb b/activesupport/lib/active_support/core_ext/object/deep_dup.rb index c66c5eb2d9..5c903917f5 100644 --- a/activesupport/lib/active_support/core_ext/object/deep_dup.rb +++ b/activesupport/lib/active_support/core_ext/object/deep_dup.rb @@ -43,7 +43,7 @@ class Hash def deep_dup hash = dup each_pair do |key, value| - if key.frozen? && ::String === key + if (::String === key && key.frozen?) || ::Symbol === key hash[key] = value.deep_dup else hash.delete(key) diff --git a/activesupport/test/core_ext/object/deep_dup_test.rb b/activesupport/test/core_ext/object/deep_dup_test.rb index 1fef3129c6..a4fa00f5f2 100644 --- a/activesupport/test/core_ext/object/deep_dup_test.rb +++ b/activesupport/test/core_ext/object/deep_dup_test.rb @@ -56,4 +56,14 @@ class DeepDupTest < ActiveSupport::TestCase dup = hash.deep_dup assert_equal 1, dup.keys.length end + + def test_deep_dup_with_mutable_frozen_key + key = { array: [] }.freeze + hash = { key => :value } + + dup = hash.deep_dup + dup.transform_keys { |k| k[:array] << :array_element } + + assert_not_equal hash.keys, dup.keys + end end