Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance on enforce_available_locales! #249

Merged

Conversation

arthurnn
Copy link
Contributor

@arthurnn arthurnn commented Apr 7, 2014

Cache the available_locales in a local Set, so we can lookup, and check for inclusions faster.
[fixes #230]

review @carlosantoniodasilva

@@ -41,10 +41,18 @@ def available_locales
@@available_locales || backend.available_locales
end

def available_locales_set
@@available_locales_set ||= available_locales.inject(Set.new) do |set, locale|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to trade off space for performance in here, So I am adding 2 entries for each locale. (which should be super small anyways). Thus, we dont need to cast to string when looking up.

Cache the available_locales in a local Set, so we can lookup, and check for inclusions faster.
[fixes ruby-i18n#230]
@arthurnn
Copy link
Contributor Author

arthurnn commented Apr 7, 2014

https://gist.github.com/arthurnn/10031665 gist to check the benchmark regression, and fix.

@arthurnn
Copy link
Contributor Author

arthurnn commented Apr 9, 2014

IPS results:

before the regression:(version 0.6.5)

Calculating -------------------------------------
                          6809 i/100ms
-------------------------------------------------
                        77460.3 (±3.4%) i/s -     388113 in   5.017347s

after the regression:(version 0.6.8)

Calculating -------------------------------------
                          3484 i/100ms
-------------------------------------------------
                        37268.7 (±3.4%) i/s -     188136 in   5.054946s

after this PR applied:

Calculating -------------------------------------
                          6168 i/100ms
-------------------------------------------------
                        69042.9 (±3.3%) i/s -     345408 in   5.008701s

thedarkone added a commit to thedarkone/i18n that referenced this pull request Apr 14, 2014
The validity of config.locale is inforced in Config#locale=.

Related ruby-i18n#230 and ruby-i18n#249.
@arthurnn
Copy link
Contributor Author

ping @carlosantoniodasilva

thedarkone added a commit to thedarkone/i18n that referenced this pull request Apr 18, 2014
The validity of config.locale is inforced in Config#locale=.

Related ruby-i18n#230 and ruby-i18n#249.
@carlosantoniodasilva carlosantoniodasilva self-assigned this May 2, 2014
@carlosantoniodasilva carlosantoniodasilva merged commit a65fffd into ruby-i18n:master May 7, 2014
carlosantoniodasilva added a commit that referenced this pull request May 7, 2014
Improve performance on enforce_available_locales!
@carlosantoniodasilva
Copy link
Member

Thanks @arthurnn ❤️

@arthurnn arthurnn deleted the available_locales_perf branch May 7, 2014 13:23
thedarkone added a commit to thedarkone/i18n that referenced this pull request Jun 30, 2014
The validity of config.locale is inforced in Config#locale=.

Related ruby-i18n#230 and ruby-i18n#249.
thedarkone added a commit to thedarkone/i18n that referenced this pull request Jun 30, 2014
The validity of config.locale is inforced in Config#locale=.

Related ruby-i18n#230 and ruby-i18n#249.
thedarkone added a commit to thedarkone/i18n that referenced this pull request Jun 30, 2014
The validity of config.locale is inforced in Config#locale=.

Related ruby-i18n#230 and ruby-i18n#249.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Major performance regression with enforce_available_locales = true
3 participants