Optimize Backend::Simple#available_locales #406
Conversation
Previously available_locales was a little bit slow. For each locale in
translations it would:
* build a new array of all the top-level keys in that locale
* build a second array of those keys except :i18n
* add that locale to the list if the second array was not empty
For locales with many translations this can build somewhat sizeable
arrays.
Instead we can perform the same operation, rejecting locales with either
no keys or only :i18n without allocating any new objects. We reject
based on the condition:
data.size <= 1 && (data.empty? || data.has_key?(:i18n))
This ends up being about 4x faster (though this of course depends on
the exact locales being used):
Benchmark.ips do |x|
x.report("I18n.available_locales") do
I18n.available_locales
end
end
Before:
11.447k (± 2.9%) i/s - 57.869k in 5.059738s
After:
47.810k (± 2.8%) i/s - 242.060k in 5.067332s
| @@ -44,7 +44,7 @@ def store_translations(locale, data, options = {}) | |||
| def available_locales | |||
| init_translations unless initialized? | |||
| translations.inject([]) do |locales, (locale, data)| | |||
| locales << locale unless (data.keys - [:i18n]).empty? | |||
| locales << locale unless data.size <= 1 && (data.empty? || data.has_key?(:i18n)) | |||
There was a problem hiding this comment.
Please replace with if condition to make it more readable: if data.size > 1 || !(data.empty? || data.has_key?(:i18n))
There was a problem hiding this comment.
I'd be happy to change it, but I'm not sure that improves readability. My mental model of this block is a reject we are implementing with inject (so that we get easy locale, data pairs) and that maps best to unless.
Does an i18n maintainer have a preference?
There was a problem hiding this comment.
Hi :) I am an i18n maintainer with an opinion on this topic.
I think unless is fine. I think the intention is clear here.
|
Many thanks @jhawthorn :) Glad to see you're still alive and kicking! |
|
I should also mention: this change will go out in 0.9.4. I am waiting on #407 to be confirmed before doing that release. |
Previously available_locales was a little bit slow. For each locale in translations it would:
For locales with many translations this can build somewhat sizeable arrays.
Instead we can perform the same operation, rejecting locales with either no keys or only :i18n without allocating any new objects. We reject based on the condition:
This ends up being about 4x faster (though this of course depends on the exact locales being used):
Before:
After: