Performance changes #138
Performance changes #138
Conversation
…tually run into one.
The validity of config.locale is inforced in Config#locale=. Related ruby-i18n#230 and ruby-i18n#249.
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
```
|
@carlosantoniodasilva 😞 I'm sorry for pinging you about this, but |
there is now some unnecessary 
|
@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 ❤️. |
|
My plan is to basically do what this PR does, so we should merge this (after we can make the build green). |
|
@tenderlove you mean the build on this branch? Because master is supposed to be green now. |
|
@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. |
|
👍 I'll merge them later ❤️ |
Performance changes Conflicts: lib/i18n.rb lib/i18n/backend/chain.rb
|
@thedarkone / @tenderlove any interest in getting this PR across the line for i18n 0.8.0? |
|
@radar what is the timetable? Would it be acceptable if I would need maybe up to 3-4 weeks time to look at it? |
|
@thedarkone I don't have any particular timetable in mind yet. The i18n gem hasn't received a version bump since Dec '14. It can wait a little while longer. If you need 3-4 weeks to complete it, then that's OK with me. 👍 |
|
Bumping this to 0.9.0. Looking to prep a 0.8.0 beta release for December, with a 0.9.0 next quarter. |
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.