Skip to content

Commit

Permalink
No longer rely on refinements for Hash utility methods.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
byroot committed Aug 24, 2021
1 parent 3d0f144 commit 4dbe501
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 131 deletions.
1 change: 0 additions & 1 deletion Gemfile
Expand Up @@ -7,5 +7,4 @@ gem 'test_declarative', '0.0.6'
gem 'rake', '~> 13'
gem 'minitest', '~> 5.14'
gem 'json'
gem 'activesupport'
gem 'pry'
1 change: 1 addition & 0 deletions lib/i18n.rb
Expand Up @@ -4,6 +4,7 @@
require 'concurrent/hash'

require 'i18n/version'
require 'i18n/utils'
require 'i18n/exceptions'
require 'i18n/interpolate/ruby'

Expand Down
4 changes: 1 addition & 3 deletions lib/i18n/backend/base.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions lib/i18n/backend/chain.rb
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
4 changes: 1 addition & 3 deletions lib/i18n/backend/gettext.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 4 additions & 6 deletions lib/i18n/backend/key_value.rb
Expand Up @@ -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

Expand All @@ -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"
Expand All @@ -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
Expand All @@ -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
Expand Down
6 changes: 2 additions & 4 deletions lib/i18n/backend/simple.rb
Expand Up @@ -19,8 +19,6 @@ module Backend
#
# I18n::Backend::Simple.include(I18n::Backend::Pluralization)
class Simple
using I18n::HashRefinements

module Implementation
include Base

Expand All @@ -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
Expand Down
59 changes: 0 additions & 59 deletions lib/i18n/core_ext/hash.rb

This file was deleted.

53 changes: 53 additions & 0 deletions lib/i18n/utils.rb
@@ -0,0 +1,53 @@
# frozen_string_literal: true

module I18n
module Utils
module_function

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

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
51 changes: 0 additions & 51 deletions test/core_ext/hash_test.rb

This file was deleted.

30 changes: 30 additions & 0 deletions 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

0 comments on commit 4dbe501

Please sign in to comment.