From 3104705b8e6dab462fe537dbb0d88c1c01635bf5 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 24 Aug 2021 13:02:50 +0200 Subject: [PATCH] No longer rely on refinements for Hash utility methods. Refinements have a very significant performance overhead. The simple fact of refining a method, even if the refinement is never used will make calls to that method 40% slower. On rarely called methods in doesn't matter that much, but I18n refine some popular Active Support methods, so any project including the I18n gem suffer from a significant performance penalty. See this benchmark for more details: https://gist.github.com/casperisfine/1c46f05cccfa945cd156f445f0d3d6fa In the end these refinements are easily replace by simple module functions taking one extra argument. --- Gemfile | 1 - lib/i18n.rb | 1 + lib/i18n/backend/base.rb | 4 +-- lib/i18n/backend/chain.rb | 6 ++-- lib/i18n/backend/gettext.rb | 4 +-- lib/i18n/backend/key_value.rb | 10 +++--- lib/i18n/backend/simple.rb | 6 ++-- lib/i18n/core_ext/hash.rb | 59 ----------------------------------- lib/i18n/utils.rb | 55 ++++++++++++++++++++++++++++++++ test/core_ext/hash_test.rb | 51 ------------------------------ test/utils_test.rb | 30 ++++++++++++++++++ 11 files changed, 96 insertions(+), 131 deletions(-) delete mode 100644 lib/i18n/core_ext/hash.rb create mode 100644 lib/i18n/utils.rb delete mode 100644 test/core_ext/hash_test.rb create mode 100644 test/utils_test.rb diff --git a/Gemfile b/Gemfile index 6bd7f3a5..b81e865e 100644 --- a/Gemfile +++ b/Gemfile @@ -7,5 +7,4 @@ gem 'test_declarative', '0.0.6' gem 'rake', '~> 13' gem 'minitest', '~> 5.14' gem 'json' -gem 'activesupport' gem 'pry' diff --git a/lib/i18n.rb b/lib/i18n.rb index 237bfb5e..152b954b 100644 --- a/lib/i18n.rb +++ b/lib/i18n.rb @@ -4,6 +4,7 @@ require 'concurrent/hash' require 'i18n/version' +require 'i18n/utils' require 'i18n/exceptions' require 'i18n/interpolate/ruby' diff --git a/lib/i18n/backend/base.rb b/lib/i18n/backend/base.rb index b04a259e..cf8f81c2 100644 --- a/lib/i18n/backend/base.rb +++ b/lib/i18n/backend/base.rb @@ -2,12 +2,10 @@ require 'yaml' require 'json' -require 'i18n/core_ext/hash' module I18n module Backend module Base - using I18n::HashRefinements include I18n::Backend::Transliterator # Accepts a list of paths to translation files. Loads translations from @@ -53,7 +51,7 @@ def translate(locale, key, options = EMPTY_HASH) end deep_interpolation = options[:deep_interpolation] - values = options.except(*RESERVED_KEYS) + values = Utils.except(options, *RESERVED_KEYS) if values entry = if deep_interpolation deep_interpolate(locale, entry, values) diff --git a/lib/i18n/backend/chain.rb b/lib/i18n/backend/chain.rb index 9ab85753..525dd2de 100644 --- a/lib/i18n/backend/chain.rb +++ b/lib/i18n/backend/chain.rb @@ -17,8 +17,6 @@ module Backend # The implementation assumes that all backends added to the Chain implement # a lookup method with the same API as Simple backend does. class Chain - using I18n::HashRefinements - module Implementation include Base @@ -55,7 +53,7 @@ def available_locales def translate(locale, key, default_options = EMPTY_HASH) namespace = nil - options = default_options.except(:default) + options = Utils.except(default_options, :default) backends.each do |backend| catch(:exception) do @@ -101,7 +99,7 @@ def translations init_translations unless initialized? translations end - memo.deep_merge!(partial_translations) { |_, a, b| b || a } + Utils.deep_merge!(memo, partial_translations) { |_, a, b| b || a } end end diff --git a/lib/i18n/backend/gettext.rb b/lib/i18n/backend/gettext.rb index 72d20f06..93a43637 100644 --- a/lib/i18n/backend/gettext.rb +++ b/lib/i18n/backend/gettext.rb @@ -31,8 +31,6 @@ module Backend # Without it strings containing periods (".") will not be translated. module Gettext - using I18n::HashRefinements - class PoData < Hash def set_comment(msgid_or_sym, comment) # ignore @@ -61,7 +59,7 @@ def normalize(locale, data) { part => _normalized.empty? ? value : _normalized } end - result.deep_merge!(normalized) + Utils.deep_merge!(result, normalized) end result end diff --git a/lib/i18n/backend/key_value.rb b/lib/i18n/backend/key_value.rb index e391672b..b937e253 100644 --- a/lib/i18n/backend/key_value.rb +++ b/lib/i18n/backend/key_value.rb @@ -67,8 +67,6 @@ module Backend # # This is useful if you are using a KeyValue backend chained to a Simple backend. class KeyValue - using I18n::HashRefinements - module Implementation attr_accessor :store @@ -91,7 +89,7 @@ def store_translations(locale, data, options = EMPTY_HASH) when Hash if @subtrees && (old_value = @store[key]) old_value = JSON.decode(old_value) - value = old_value.deep_symbolize_keys.deep_merge!(value) if old_value.is_a?(Hash) + value = Utils.deep_merge!(Utils.deep_symbolize_keys(old_value), value) if old_value.is_a?(Hash) end when Proc raise "Key-value stores cannot handle procs" @@ -115,12 +113,12 @@ def available_locales # them into a hash such as the one returned from loading the # haml files def translations - @translations = @store.keys.clone.map do |main_key| + @translations = Utils.deep_symbolize_keys(@store.keys.clone.map do |main_key| main_value = JSON.decode(@store[main_key]) main_key.to_s.split(".").reverse.inject(main_value) do |value, key| {key.to_sym => value} end - end.inject{|hash, elem| hash.deep_merge!(elem)}.deep_symbolize_keys + end.inject{|hash, elem| Utils.deep_merge!(hash, elem)}) end def init_translations @@ -141,7 +139,7 @@ def lookup(locale, key, scope = [], options = EMPTY_HASH) value = JSON.decode(value) if value if value.is_a?(Hash) - value.deep_symbolize_keys + Utils.deep_symbolize_keys(value) elsif !value.nil? value elsif !@subtrees diff --git a/lib/i18n/backend/simple.rb b/lib/i18n/backend/simple.rb index 0dde82d7..fe0ef863 100644 --- a/lib/i18n/backend/simple.rb +++ b/lib/i18n/backend/simple.rb @@ -19,8 +19,6 @@ module Backend # # I18n::Backend::Simple.include(I18n::Backend::Pluralization) class Simple - using I18n::HashRefinements - module Implementation include Base @@ -40,8 +38,8 @@ def store_translations(locale, data, options = EMPTY_HASH) end locale = locale.to_sym translations[locale] ||= Concurrent::Hash.new - data = data.deep_symbolize_keys - translations[locale].deep_merge!(data) + data = Utils.deep_symbolize_keys(data) + Utils.deep_merge!(translations[locale], data) end # Get available locales from the translations hash diff --git a/lib/i18n/core_ext/hash.rb b/lib/i18n/core_ext/hash.rb deleted file mode 100644 index e022e842..00000000 --- a/lib/i18n/core_ext/hash.rb +++ /dev/null @@ -1,59 +0,0 @@ -module I18n - module HashRefinements - refine Hash do - using I18n::HashRefinements - def except(*keys) - dup.except!(*keys) - end unless method_defined?(:except) - - def except!(*keys) - keys.each { |key| delete(key) } - self - end - - def deep_symbolize_keys - each_with_object({}) do |(key, value), result| - result[symbolize_key(key)] = deep_symbolize_keys_in_object(value) - result - end - end - - # deep_merge from activesupport 5 - # Copyright (c) 2005-2019 David Heinemeier Hansson - def deep_merge(other_hash, &block) - dup.deep_merge!(other_hash, &block) - end - - # deep_merge! from activesupport 5 - # Copyright (c) 2005-2019 David Heinemeier Hansson - def deep_merge!(other_hash, &block) - merge!(other_hash) do |key, this_val, other_val| - if this_val.is_a?(Hash) && other_val.is_a?(Hash) - this_val.deep_merge(other_val, &block) - elsif block_given? - block.call(key, this_val, other_val) - else - other_val - end - end - end - - def symbolize_key(key) - key.respond_to?(:to_sym) ? key.to_sym : key - end - - private - - def deep_symbolize_keys_in_object(value) - case value - when Hash - value.deep_symbolize_keys - when Array - value.map { |e| deep_symbolize_keys_in_object(e) } - else - value - end - end - end - end -end diff --git a/lib/i18n/utils.rb b/lib/i18n/utils.rb new file mode 100644 index 00000000..88415615 --- /dev/null +++ b/lib/i18n/utils.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module I18n + module Utils + class << self + if Hash.method_defined?(:except) + def except(hash, *keys) + hash.except(*keys) + end + else + def except(hash, *keys) + hash = hash.dup + keys.each { |k| hash.delete(k) } + hash + end + end + + def deep_merge(hash, other_hash, &block) + deep_merge!(hash.dup, other_hash, &block) + end + + def deep_merge!(hash, other_hash, &block) + hash.merge!(other_hash) do |key, this_val, other_val| + if this_val.is_a?(Hash) && other_val.is_a?(Hash) + deep_merge(this_val, other_val, &block) + elsif block_given? + yield key, this_val, other_val + else + other_val + end + end + end + + def deep_symbolize_keys(hash) + hash.each_with_object({}) do |(key, value), result| + result[key.respond_to?(:to_sym) ? key.to_sym : key] = deep_symbolize_keys_in_object(value) + result + end + end + + private + + def deep_symbolize_keys_in_object(value) + case value + when Hash + deep_symbolize_keys(value) + when Array + value.map { |e| deep_symbolize_keys_in_object(e) } + else + value + end + end + end + end +end diff --git a/test/core_ext/hash_test.rb b/test/core_ext/hash_test.rb deleted file mode 100644 index ada13f57..00000000 --- a/test/core_ext/hash_test.rb +++ /dev/null @@ -1,51 +0,0 @@ -require 'test_helper' -require 'i18n/core_ext/hash' -require 'active_support/core_ext/hash' - -class I18nCoreExtHashInterpolationTest < I18n::TestCase - using I18n::HashRefinements - - test "#deep_symbolize_keys" do - hash = { 'foo' => { 'bar' => { 'baz' => 'bar' } } } - expected = { :foo => { :bar => { :baz => 'bar' } } } - assert_equal expected, hash.deep_symbolize_keys - end - - test "#deep_symbolize_keys with numeric keys" do - hash = { 1 => { 2 => { 3 => 'bar' } } } - expected = { 1 => { 2 => { 3 => 'bar' } } } - assert_equal expected, hash.deep_symbolize_keys - end - - test "#slice" do - hash = { :foo => 'bar', :baz => 'bar' } - expected = { :foo => 'bar' } - assert_equal expected, hash.slice(:foo) - end - - test "#slice non-existent key" do - hash = { :foo => 'bar', :baz => 'bar' } - expected = { :foo => 'bar' } - assert_equal expected, hash.slice(:foo, :not_here) - end - - test "#except" do - hash = { :foo => 'bar', :baz => 'bar' } - expected = { :foo => 'bar' } - assert_equal expected, hash.except(:baz) - end - - test "#deep_merge!" do - hash = { :foo => { :bar => { :baz => 'bar' } }, :baz => 'bar' } - hash.deep_merge!(:foo => { :bar => { :baz => 'foo' } }) - - expected = { :foo => { :bar => { :baz => 'foo' } }, :baz => 'bar' } - assert_equal expected, hash - end - - test "#except supports ActiveSupport::HashWithIndifferentAccess" do - hash = { :foo => 'bar', :baz => 'bar' }.with_indifferent_access - expected = { :foo => 'bar' }.with_indifferent_access - assert_equal expected, hash.except(:baz) - end -end diff --git a/test/utils_test.rb b/test/utils_test.rb new file mode 100644 index 00000000..21ea4765 --- /dev/null +++ b/test/utils_test.rb @@ -0,0 +1,30 @@ +require 'test_helper' + +class I18nUtilsTest < I18n::TestCase + + test ".deep_symbolize_keys" do + hash = { 'foo' => { 'bar' => { 'baz' => 'bar' } } } + expected = { :foo => { :bar => { :baz => 'bar' } } } + assert_equal expected, I18n::Utils.deep_symbolize_keys(hash) + end + + test "#deep_symbolize_keys with numeric keys" do + hash = { 1 => { 2 => { 3 => 'bar' } } } + expected = { 1 => { 2 => { 3 => 'bar' } } } + assert_equal expected, I18n::Utils.deep_symbolize_keys(hash) + end + + test "#except" do + hash = { :foo => 'bar', :baz => 'bar' } + expected = { :foo => 'bar' } + assert_equal expected, I18n::Utils.except(hash, :baz) + end + + test "#deep_merge!" do + hash = { :foo => { :bar => { :baz => 'bar' } }, :baz => 'bar' } + I18n::Utils.deep_merge!(hash, :foo => { :bar => { :baz => 'foo' } }) + + expected = { :foo => { :bar => { :baz => 'foo' } }, :baz => 'bar' } + assert_equal expected, hash + end +end