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

Lower gc pressure #6

Merged
merged 5 commits into from
Sep 9, 2016
Merged

Conversation

doits
Copy link
Contributor

@doits doits commented Sep 9, 2016

By using the missing_interpolation_argument_handler and not always merging the globals with local interpolation values, it does not create a new hash for each call anymore. Lowers GC pressure noticeable.

This adds some complexity in favour of performance ... what do you say @attilahorvath?

Created on top of #5 and #7, so when they are merged, this is cleaner.

@doits doits force-pushed the lower_gc_pressure branch 2 times, most recently from 9ca6c64 to 1170535 Compare September 9, 2016 13:23
@doits
Copy link
Contributor Author

doits commented Sep 9, 2016

By using prepend we have no duplicated code, but it requires Ruby >= 2. Are you OK to drop Ruby < 2 support?

Not sure why `reject` gives warnings, `select` not ...
By using the `missing_interpolation_argument_handler` and not always
merging the globals with local interpolation values, it does not create
a new hash for each call anymore. Lowers GC pressure noticable.
Because: No copy and pasted code from I18n
@doits doits force-pushed the lower_gc_pressure branch from fd9ea85 to ab99bfd Compare September 9, 2016 13:50
@doits doits force-pushed the lower_gc_pressure branch from ab99bfd to a8b8472 Compare September 9, 2016 13:50
@attilahorvath
Copy link
Owner

Well, Ruby 2 is not supported anymore anyway, so I think it's fine. I'll make a point about it in the Readme though.

@attilahorvath
Copy link
Owner

Alright, great.

@attilahorvath attilahorvath merged commit 6ff3101 into attilahorvath:master Sep 9, 2016
thabemmz added a commit to Mobiliteitsfabriek/i18n-globals that referenced this pull request Mar 16, 2023
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.

2 participants