Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Performance changes #138

Open
wants to merge 12 commits into from

3 participants

@thedarkone

I18n started showing up in perf profiles for my app again. I'm no longer that much familiar with the I18n's codebase as I used to be, so some code review is really welcome.

@thedarkone thedarkone referenced this pull request from a commit in tenderlove/i18n
@tenderlove tenderlove stop using throw / catch for exceptions
This cleans up our code, and makes it faster.  Every call to `translate`
required an `is_a` check to make sure that the return value wasn't a
translation error.  This behavior punished method calls when the lookup
was successful.  Successful lookup should *hopefully* be the majority of
cases, so we should make that code path as fast as possible.

Benchmark:

```ruby
require 'i18n'
require 'benchmark/ips'
require 'set'

I18n.backend = I18n::Backend::Simple.new
I18n.backend.store_translations('en', :foo => 'bar')
I18n.config.enforce_available_locales = true

Benchmark.ips do |x|
  options = { :locale => 'en' }
  x.report 'tr' do
    I18n.translate(:foo, options)
  end
end
```

Before:

```
[aaron@higgins i18n (rm_throw)]$ ruby -I lib test.rb
Calculating -------------------------------------
                  tr      7661 i/100ms
-------------------------------------------------
                  tr    89120.2 (±5.0%) i/s -     451999 in   5.085290s
```

After:

```
[aaron@higgins i18n (rm_throw)]$ ruby -I lib test.rb
Calculating -------------------------------------
                  tr      8207 i/100ms
-------------------------------------------------
                  tr    93770.9 (±5.1%) i/s -     467799 in   5.002582s
```
acf0546
@thedarkone

@carlosantoniodasilva :disappointed: I'm sorry for pinging you about this, but :bowtie: there is now some unnecessary tenderlove@acf0546 duplicate work by @tenderlove going on. Please have a look at this when you have the time :heart:.

@carlosantoniodasilva
Collaborator

@thedarkone no problem :), I'm aware that @tenderlove did some work on i18n yesterday, but I want to talk to him first to understand what are his plans. I have another branch with tons of changes and removals due to old Ruby/Rails compatibilities that I also want to get in, but I haven't had time to work on OSS these days due to personal stuff, which is something that should be solved by next week. I'll do my best then to get all this in, and I'll talk to @tenderlove meanwhile. Thanks :heart:.

@tenderlove

My plan is to basically do what this PR does, so we should merge this (after we can make the build green).

@carlosantoniodasilva
Collaborator

@tenderlove you mean the build on this branch? Because master is supposed to be green now.

@tenderlove

@carlosantoniodasilva I mean this branch. :-)

But, there are some tests that fail randomly on master, so I sent pull requests to deal with them. I'll pull down @thedarkone's changes and see if I can get all the tests to pass.

@carlosantoniodasilva
Collaborator

:+1: I'll merge them later :heart:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
45 lib/i18n.rb
@@ -9,7 +9,7 @@ module I18n
autoload :Locale, 'i18n/locale'
autoload :Tests, 'i18n/tests'
- RESERVED_KEYS = [:scope, :default, :separator, :resolve, :object, :fallback, :format, :cascade, :throw, :raise, :rescue_format]
+ RESERVED_KEYS = [:scope, :default, :separator, :resolve, :object, :fallback, :format, :cascade, :throw, :raise, :rescue_format, :exception_handler]
RESERVED_KEYS_PATTERN = /%\{(#{RESERVED_KEYS.join("|")})\}/
extend(Module.new {
@@ -140,24 +140,29 @@ def reload!
# from the argument values passed to #translate. Therefor your lambdas should
# always return the same translations/values per unique combination of argument
# values.
- def translate(*args)
- options = args.last.is_a?(Hash) ? args.pop.dup : {}
- key = args.shift
- backend = config.backend
- locale = options.delete(:locale) || config.locale
- handling = options.delete(:throw) && :throw || options.delete(:raise) && :raise # TODO deprecate :raise
+ def translate(key, options=nil)
+ if key.is_a?(Hash)
+ options = key
+ key = nil
+ end
- enforce_available_locales!(locale)
- raise I18n::ArgumentError if key.is_a?(String) && key.empty?
+ options = options.dup if options
+ conf = config
+ backend = conf.backend
- result = catch(:exception) do
- if key.is_a?(Array)
- key.map { |k| backend.translate(locale, k, options) }
- else
- backend.translate(locale, key, options)
- end
+ if locale = options && options.delete(:locale)
+ enforce_available_locales!(locale)
+ else
+ locale = conf.locale
end
- result.is_a?(MissingTranslation) ? handle_exception(handling, result, locale, key, options) : result
+
+ result = if key.is_a?(Array)
+ key.map { |k| backend.translate(locale, k, options) }
+ else
+ backend.translate(locale, key, options)
+ end
+
+ nil == result ? handle_exception(I18n::MissingTranslation.new(locale, key, options), locale, key, options) : result
end
alias :t :translate
@@ -229,12 +234,11 @@ def transliterate(*args)
options = args.pop.dup if args.last.is_a?(Hash)
key = args.shift
locale = options && options.delete(:locale) || config.locale
- handling = options && (options.delete(:throw) && :throw || options.delete(:raise) && :raise)
replacement = options && options.delete(:replacement)
enforce_available_locales!(locale)
config.backend.transliterate(locale, key, replacement)
rescue I18n::ArgumentError => exception
- handle_exception(handling, exception, locale, key, options || {})
+ handle_exception(exception, locale, key, options || {})
end
# Localizes certain objects, such as dates and numbers to local formatting.
@@ -308,8 +312,9 @@ def enforce_available_locales!(locale)
#
# I18n.exception_handler = I18nExceptionHandler.new # an object
# I18n.exception_handler.call(exception, locale, key, options) # will be called like this
- def handle_exception(handling, exception, locale, key, options)
- case handling
+ def handle_exception(exception, locale, key, options)
+ options ||= {}
+ case options.delete(:throw) && :throw || options.delete(:raise) && :raise # TODO deprecate :raise
when :raise
raise(exception.respond_to?(:to_exception) ? exception.to_exception : exception)
when :throw
View
71 lib/i18n/backend/base.rb
@@ -7,6 +7,11 @@ module Backend
module Base
include I18n::Backend::Transliterator
+ RESERVED_KEYS_HASH = RESERVED_KEYS.inject({}) do |h, reserved_key|
+ h[reserved_key] = h[reserved_key.to_s.freeze] = true
+ h
+ end
+
# Accepts a list of paths to translation files. Loads translations from
# plain Ruby (*.rb) or YAML files (*.yml). See #load_rb and #load_yml
# for details.
@@ -21,25 +26,25 @@ def store_translations(locale, data, options = {})
raise NotImplementedError
end
- def translate(locale, key, options = {})
+ def translate(locale, key, options = nil)
raise InvalidLocale.new(locale) unless locale
- entry = key && lookup(locale, key, options[:scope], options)
+ entry = key && lookup(locale, key, options && options[:scope], options)
- if options.empty?
- entry = resolve(locale, key, entry, options)
- else
- count, default = options.values_at(:count, :default)
- values = options.except(*RESERVED_KEYS)
- entry = entry.nil? && default ?
+ if options
+ count = options[:count]
+ default = options[:default]
+ entry = nil == entry && default ?
default(locale, key, default, options) : resolve(locale, key, entry, options)
+ else
+ entry = resolve(locale, key, entry, options)
end
- throw(:exception, I18n::MissingTranslation.new(locale, key, options)) if entry.nil?
- entry = entry.dup if entry.is_a?(String)
-
- entry = pluralize(locale, entry, count) if count
- entry = interpolate(locale, entry, values) if values
- entry
+ unless nil == entry
+ entry = entry.dup if entry.is_a?(String)
+ entry = pluralize(locale, entry, count) if count
+ entry = interpolate(locale, entry, options) if options
+ entry
+ end
end
def exists?(locale, key)
@@ -100,7 +105,9 @@ def default(locale, object, subject, options = {})
case subject
when Array
subject.each do |item|
- result = resolve(locale, object, item, options) and return result
+ unless nil == (result = resolve(locale, object, item, options))
+ return result
+ end
end and nil
else
resolve(locale, object, subject, options)
@@ -111,20 +118,19 @@ def default(locale, object, subject, options = {})
# If the given subject is a Symbol, it will be translated with the
# given options. If it is a Proc then it will be evaluated. All other
# subjects will be returned directly.
- def resolve(locale, object, subject, options = {})
- return subject if options[:resolve] == false
- result = catch(:exception) do
- case subject
- when Symbol
- I18n.translate(subject, options.merge(:locale => locale, :throw => true))
- when Proc
- date_or_time = options.delete(:object) || object
- resolve(locale, object, subject.call(date_or_time, options))
- else
- subject
- end
+ def resolve(locale, object, subject, options = nil)
+ return subject if options && options[:resolve] == false
+
+ case subject
+ when Symbol
+ overrides = {:locale => locale, :exception_handler => DoNothingExceptionHandler}
+ I18n.translate(subject, options ? options.merge(overrides) : overrides)
+ when Proc
+ date_or_time = (options && options.delete(:object)) || object
+ resolve(locale, object, subject.call(date_or_time, options))
+ else
+ subject
end
- result unless result.is_a?(MissingTranslation)
end
# Picks a translation from a pluralized mnemonic subkey according to English
@@ -149,13 +155,20 @@ def pluralize(locale, entry, count)
# interpolate "file %{file} opened by %%{user}", :file => 'test.txt', :user => 'Mr. X'
# # => "file test.txt opened by %{user}"
def interpolate(locale, string, values = {})
- if string.is_a?(::String) && !values.empty?
+ if string.is_a?(::String) && interpolation_values_any?(values)
I18n.interpolate(string, values)
else
string
end
end
+ def interpolation_values_any?(values)
+ values.each_key do |key|
+ return true if !RESERVED_KEYS_HASH[key]
+ end
+ false
+ end
+
# Loads a single translations file by delegating to #load_rb or
# #load_yml depending on the file extension and directly merges the
# data to the existing translations. Raises I18n::UnknownFileType
View
7 lib/i18n/backend/cache.rb
@@ -55,7 +55,7 @@ def cache_namespace=(namespace)
end
def perform_caching?
- !cache_store.nil?
+ nil != cache_store
end
end
@@ -70,14 +70,13 @@ def translate(locale, key, options = {})
def fetch(cache_key, &block)
result = _fetch(cache_key, &block)
- throw(:exception, result) if result.is_a?(MissingTranslation)
result = result.dup if result.frozen? rescue result
result
end
- def _fetch(cache_key, &block)
+ def _fetch(cache_key)
result = I18n.cache_store.read(cache_key) and return result
- result = catch(:exception, &block)
+ result = yield
I18n.cache_store.write(cache_key, result) unless result.is_a?(Proc)
result
end
View
8 lib/i18n/backend/cascade.rb
@@ -31,13 +31,13 @@
module I18n
module Backend
module Cascade
- def lookup(locale, key, scope = [], options = {})
- return super unless cascade = options[:cascade]
+ def lookup(locale, key, scope = [], options = nil)
+ return super unless cascade = options && options[:cascade]
cascade = { :step => 1 } unless cascade.is_a?(Hash)
step = cascade[:step] || 1
offset = cascade[:offset] || 1
- separator = options[:separator] || I18n.default_separator
+ separator = (options && options[:separator]) || I18n.default_separator
skip_root = cascade.has_key?(:skip_root) ? cascade[:skip_root] : true
scope = I18n.normalize_keys(nil, key, scope, separator)
@@ -45,7 +45,7 @@ def lookup(locale, key, scope = [], options = {})
begin
result = super
- return result unless result.nil?
+ return result unless nil == result
scope = scope.dup
end while (!scope.empty? || !skip_root) && scope.slice!(-step, step)
end
View
31 lib/i18n/backend/chain.rb
@@ -36,24 +36,21 @@ def available_locales
backends.map { |backend| backend.available_locales }.flatten.uniq
end
- def translate(locale, key, default_options = {})
+ def translate(locale, key, default_options = nil)
namespace = nil
- options = default_options.except(:default)
+ options = default_options && default_options.except(:default)
backends.each do |backend|
- catch(:exception) do
- options = default_options if backend == backends.last
- translation = backend.translate(locale, key, options)
- if namespace_lookup?(translation, options)
- namespace = translation.merge(namespace || {})
- elsif !translation.nil?
- return translation
- end
+ options = default_options if backend == backends.last
+ translation = backend.translate(locale, key, options)
+ if namespace_lookup?(translation, options)
+ namespace = translation.merge(namespace || {})
+ elsif !translation.nil?
+ return translation
end
end
- return namespace if namespace
- throw(:exception, I18n::MissingTranslation.new(locale, key, options))
+ namespace
end
def exists?(locale, key)
@@ -64,16 +61,16 @@ def exists?(locale, key)
def localize(locale, object, format = :default, options = {})
backends.each do |backend|
- catch(:exception) do
- result = backend.localize(locale, object, format, options) and return result
+ unless nil == (result = backend.localize(locale, object, format, options))
+ return result
end
end
- throw(:exception, I18n::MissingTranslation.new(locale, format, options))
+ nil
end
protected
- def namespace_lookup?(result, options)
- result.is_a?(Hash) && !options.has_key?(:count)
+ def namespace_lookup?(result, options = nil)
+ result.is_a?(Hash) && !(options && options.has_key?(:count))
end
end
View
13 lib/i18n/backend/fallbacks.rb
@@ -34,16 +34,19 @@ module Fallbacks
# The default option takes precedence over fallback locales only when
# it's a Symbol. When the default contains a String, Proc or Hash
# it is evaluated last after all the fallback locales have been tried.
- def translate(locale, key, options = {})
+ def translate(locale, key, options = nil)
+ options ||= {}
return super if options[:fallback]
default = extract_non_symbol_default!(options) if options[:default]
options[:fallback] = true
I18n.fallbacks[locale].each do |fallback|
+ # TODO: Fallbacks should be fixed to not ever return invalid locales.
+ # The same strategy as in Memoize module can be employed to intercept
+ # store_translations to update available fallbacks.
begin
- catch(:exception) do
- result = super(fallback, key, options)
- return result unless result.nil?
+ unless nil == (result = super(fallback, key, options))
+ return result
end
rescue I18n::InvalidLocale
# we do nothing when the locale is invalid, as this is a fallback anyways.
@@ -52,7 +55,7 @@ def translate(locale, key, options = {})
options.delete(:fallback)
return super(locale, nil, options.merge(:default => default)) if default
- throw(:exception, I18n::MissingTranslation.new(locale, key, options))
+ nil
end
def extract_non_symbol_default!(options)
View
2  lib/i18n/backend/gettext.rb
@@ -39,7 +39,7 @@ def parse(filename)
def normalize(locale, data)
data.inject({}) do |result, (key, value)|
- unless key.nil? || key.empty?
+ unless nil == key || key.empty?
key = key.gsub(I18n::Gettext::CONTEXT_SEPARATOR, '|')
key, value = normalize_pluralization(locale, key, value) if key.index("\000")
View
4 lib/i18n/backend/memoize.rb
@@ -26,9 +26,9 @@ def reload!
protected
- def lookup(locale, key, scope = nil, options = {})
+ def lookup(locale, key, scope = nil, options = nil)
flat_key = I18n::Backend::Flatten.normalize_flat_keys(locale,
- key, scope, options[:separator]).to_sym
+ key, scope, options && options[:separator]).to_sym
flat_hash = memoized_lookup[locale.to_sym]
flat_hash.key?(flat_key) ? flat_hash[flat_key] : (flat_hash[flat_key] = super)
end
View
6 lib/i18n/backend/simple.rb
@@ -67,15 +67,15 @@ def translations
# nested translations hash. Splits keys or scopes containing dots
# into multiple keys, i.e. <tt>currency.format</tt> is regarded the same as
# <tt>%w(currency format)</tt>.
- def lookup(locale, key, scope = [], options = {})
+ def lookup(locale, key, scope = [], options = nil)
init_translations unless initialized?
- keys = I18n.normalize_keys(locale, key, scope, options[:separator])
+ keys = I18n.normalize_keys(locale, key, scope, options && options[:separator])
keys.inject(translations) do |result, _key|
_key = _key.to_sym
return nil unless result.is_a?(Hash) && result.has_key?(_key)
result = result[_key]
- result = resolve(locale, _key, result, options.merge(:scope => nil)) if result.is_a?(Symbol)
+ result = resolve(locale, _key, result, options && options.merge(:scope => nil)) if result.is_a?(Symbol)
result
end
end
View
9 lib/i18n/exceptions.rb
@@ -31,6 +31,12 @@ def call(exception, locale, key, options)
}
end
+ module DoNothingExceptionHandler
+ class << self
+ def call(exception, locale, key, options); end
+ end
+ end
+
class ArgumentError < ::ArgumentError; end
class InvalidLocale < ArgumentError
@@ -54,7 +60,8 @@ module Base
attr_reader :locale, :key, :options
def initialize(locale, key, options = nil)
- @key, @locale, @options = key, locale, options.dup || {}
+ options = options ? options.dup : {}
+ @key, @locale, @options = key, locale, options
options.each { |k, v| self.options[k] = v.inspect if v.is_a?(Proc) }
end
View
2  lib/i18n/locale/fallbacks.rb
@@ -65,7 +65,7 @@ def defaults=(defaults)
attr_reader :defaults
def [](locale)
- raise InvalidLocale.new(locale) if locale.nil?
+ raise InvalidLocale.new(locale) if nil == locale
locale = locale.to_sym
super || store(locale, compute(locale))
end
View
2  lib/i18n/tests/localization/procs.rb
@@ -72,7 +72,7 @@ def inspect_args(args)
when ::Date
arg.strftime('%a, %d %b %Y')
when Hash
- arg.delete(:fallback)
+ RESERVED_KEYS.each { |key| arg.delete(key) }
arg.inspect
else
arg.inspect
View
5 lib/i18n/tests/procs.rb
@@ -48,7 +48,10 @@ module Procs
protected
def filter_args(*args)
- args.map {|arg| arg.delete(:fallback) if arg.is_a?(Hash) ; arg }.inspect
+ args.grep(Hash) do |hash_arg|
+ RESERVED_KEYS.each { |key| hash_arg.delete(key) }
+ end
+ args.inspect
end
end
end
View
3  test/api/override_test.rb
@@ -33,8 +33,9 @@ def setup
end
test "make sure modules can overwrite I18n signature" do
- exception = catch(:exception) do
+ begin
@I18n.t('Hello', 'Welcome message on home page', :tokenize => true, :throw => true)
+ rescue Exception => exception
end
assert exception.message
@I18n.extend OverrideSignature
View
6 test/i18n_test.rb
@@ -166,7 +166,7 @@ def setup
end
test "translate given no locale uses the current locale" do
- I18n.backend.expects(:translate).with(:en, :foo, {})
+ I18n.backend.expects(:translate).with(:en, :foo, nil)
I18n.translate :foo
end
@@ -202,10 +202,6 @@ def setup
assert_equal "translation missing: en.bogus", I18n.t(:bogus)
end
- test "translate given an empty string as a key raises an I18n::ArgumentError" do
- assert_raise(I18n::ArgumentError) { I18n.t("") }
- end
-
test "translate given an unavailable locale rases an I18n::InvalidLocale" do
begin
I18n.config.enforce_available_locales = true
Something went wrong with that request. Please try again.