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

No longer rely on refinements for Hash utility methods. #573

Merged
merged 1 commit into from Nov 7, 2021

Conversation

casperisfine
Copy link

Refinements have a very significant performance overhead. The simple fact of refining a method, even if the refinement is never used will make calls to that method 40% slower.

On rarely called methods in doesn't matter that much, but I18n refine some popular Active Support methods, so any project including the I18n gem suffer from a significant performance penalty.

See this benchmark for more details: https://gist.github.com/casperisfine/1c46f05cccfa945cd156f445f0d3d6fa

In the end these refinements are easily replace by simple module functions taking one extra argument.

lib/i18n/utils.rb Outdated Show resolved Hide resolved
Refinements have a very significant performance overhead.
The simple fact of refining a method, even if the refinement
is never used will make calls to that method 40% slower.

On rarely called methods in doesn't matter that much, but I18n
refine some popular Active Support methods, so any project including
the I18n gem suffer from a significant performance penalty.

See this benchmark for more details:
https://gist.github.com/casperisfine/1c46f05cccfa945cd156f445f0d3d6fa

In the end these refinements are easily replace by simple module functions
taking one extra argument.
@casperisfine
Copy link
Author

@radar any chance this PR could be considered?

@radar
Copy link
Collaborator

radar commented Nov 7, 2021

Thank you @casperisfine, this looks great.

@radar radar merged commit 3e016c8 into ruby-i18n:master Nov 7, 2021
mpantel added a commit to mpantel/i18n-active_record that referenced this pull request Jan 27, 2022
@mpantel
Copy link

mpantel commented Jan 27, 2022

There is still some issue because i18n/core/hash still gets called in:
lib/i18n/backend/chain.rb
and
lib/i18n/backend/gettext.rb
and rails fails to load:
`require': cannot load such file -- i18n/core_ext/hash (LoadError)

while Utils is correclty referenced in those files...

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.

None yet

5 participants