Skip to content

Commit

Permalink
stop using throw / catch for exceptions
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
tenderlove committed Jun 30, 2014
1 parent 750c3c3 commit acf0546
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 47 deletions.
23 changes: 10 additions & 13 deletions lib/i18n.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,22 +150,19 @@ def translate(*args)
enforce_available_locales!(locale)
raise I18n::ArgumentError if key.is_a?(String) && key.empty?

result = catch(:exception) do
if key.is_a?(Array)
key.map { |k| backend.translate(locale, k, options) }
else
begin
backend.translate(locale, key, options)
rescue ThrowException => ex
if handling == :throw
raise ex
else
return handle_exception(handling, ex.ex, locale, key, options)
end
if key.is_a?(Array)
key.map { |k| backend.translate(locale, k, options) }
else
begin
backend.translate(locale, key, options)
rescue ThrowException => ex
if handling == :throw
raise ex
else
return handle_exception(handling, ex.ex, locale, key, options)
end
end
end
result.is_a?(MissingTranslation) ? handle_exception(handling, result, locale, key, options) : result
end
alias :t :translate

Expand Down
26 changes: 11 additions & 15 deletions lib/i18n/backend/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ def translate(locale, key, options = {})
default(locale, key, default, options) : resolve(locale, key, entry, options)
end

#throw(:exception, I18n::MissingTranslation.new(locale, key, options)) if entry.nil?
raise(ThrowException.new(I18n::MissingTranslation.new(locale, key, options))) if entry.nil?
entry = entry.dup if entry.is_a?(String)

Expand Down Expand Up @@ -114,22 +113,19 @@ def default(locale, object, subject, options = {})
# 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
begin
I18n.translate(subject, options.merge(:locale => locale, :throw => true))
rescue ThrowException => ex
ex.ex
end
when Proc
date_or_time = options.delete(:object) || object
resolve(locale, object, subject.call(date_or_time, options))
else
subject
case subject
when Symbol
begin
I18n.translate(subject, options.merge(:locale => locale, :throw => true))
rescue ThrowException => ex
return nil
end
when Proc
date_or_time = 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
Expand Down
4 changes: 2 additions & 2 deletions lib/i18n/backend/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ def translate(locale, key, options = {})

def fetch(cache_key, &block)
result = _fetch(cache_key, &block)
throw(:exception, result) if result.is_a?(MissingTranslation)
raise ThrowException.new(result)
result = result.dup if result.frozen? rescue result
result
end

def _fetch(cache_key, &block)
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
Expand Down
23 changes: 11 additions & 12 deletions lib/i18n/backend/chain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,15 @@ def translate(locale, key, default_options = {})
options = default_options.except(:default)

backends.each do |backend|
catch(:exception) do
options = default_options if backend == backends.last
begin
translation = backend.translate(locale, key, options)
if namespace_lookup?(translation, options)
namespace = translation.merge(namespace || {})
elsif !translation.nil?
return translation
end
rescue ThrowException
options = default_options if backend == backends.last
begin
translation = backend.translate(locale, key, options)
if namespace_lookup?(translation, options)
namespace = translation.merge(namespace || {})
elsif !translation.nil?
return translation
end
rescue ThrowException
end
end

Expand All @@ -67,11 +65,12 @@ def exists?(locale, key)

def localize(locale, object, format = :default, options = {})
backends.each do |backend|
catch(:exception) do
begin
result = backend.localize(locale, object, format, options) and return result
rescue ThrowException
end
end
throw(:exception, I18n::MissingTranslation.new(locale, format, options))
raise ThrowException.new(I18n::MissingTranslation.new(locale, format, options))
end

protected
Expand Down
8 changes: 3 additions & 5 deletions lib/i18n/backend/fallbacks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,16 @@ def translate(locale, key, options = {})
options[:fallback] = true
I18n.fallbacks[locale].each do |fallback|
begin
catch(:exception) do
result = super(fallback, key, options)
return result unless result.nil?
end
result = super(fallback, key, options)
return result unless result.nil?
rescue I18n::InvalidLocale, ThrowException
# we do nothing when the locale is invalid, as this is a fallback anyways.
end
end
options.delete(:fallback)

return super(locale, nil, options.merge(:default => default)) if default
throw(:exception, I18n::MissingTranslation.new(locale, key, options))
raise ThrowException.new(I18n::MissingTranslation.new(locale, key, options))
end

def extract_non_symbol_default!(options)
Expand Down

2 comments on commit acf0546

@thedarkone
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tenderlove you are probably unaware, but the whole throw/catch thing is being used because a typical Rails use-case generates a lot of missing translation lookups and generating exceptions on JRuby is somewhat expensive.

Here what I get with your (slightly modified) benchmark test.rb:

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
I18n.locale = 'en'

Benchmark.ips do |x|
  x.report 'tr' do
    I18n.translate(:foo)
  end

  x.report 'missing' do
    I18n.translate(:bar)
  end
end

When running on JRuby, on master:

                  tr   158716.4 (±3.3%) i/s -     796416 in   5.025000s
             missing    60479.3 (±5.1%) i/s -     302900 in   5.027000s

When running JRuby on rm_throw:

                  tr   167659.2 (±3.2%) i/s -     842010 in   5.028000s
             missing     8703.1 (±6.0%) i/s -      43699 in   5.043000s

As it happens, I'm also quite unhappy with i18n's performance and have a pending "performace" PR, which also kills throw/catch (replacing it with nil checks). Running the bench with my PR merged (on JRuby):

                  tr   262210.0 (±3.2%) i/s -    1315248 in   5.022000s
             missing    97795.2 (±3.5%) i/s -     489810 in   5.016000s

@tenderlove
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thedarkone awesome. I just checked your PR, I think it's great. We should make the CI green, and I'll try to get someone to merge it.

Please sign in to comment.