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

Performance improvements #130

Merged
merged 11 commits into from
Nov 3, 2020
Merged

Performance improvements #130

merged 11 commits into from
Nov 3, 2020

Conversation

mlitwiniuk
Copy link
Member

This should eliminate overhead when calling same key over and over
again. I should also improve performance when accessins keys that return
nil - those were handled improperly before.

This should eliminate overhead when calling same key over and over
again. I should also improve performance when accessins keys that return
`nil` - those were handled improperly before.
@mlitwiniuk mlitwiniuk mentioned this pull request Feb 4, 2019
@mlitwiniuk mlitwiniuk self-assigned this May 31, 2020
@mlitwiniuk mlitwiniuk changed the title WIP: Performance improvements Performance improvements Nov 3, 2020
lib/lit.rb Show resolved Hide resolved
@@ -130,7 +139,7 @@ def store_item(locale, data, scope = [], startup_process = false)
# end
elsif data.respond_to?(:to_str) || data.is_a?(Array)
key = ([locale] + scope).join('.')
return if startup_process && @cache.keys.member?(key) && Lit.ignore_yaml_on_startup
return if startup_process && Lit.ignore_yaml_on_startup && (Thread.current[:lit_cache_keys] || @cache.keys).member?(key)
Copy link
Contributor

Choose a reason for hiding this comment

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

This store_item method looks too messy to me. Maybe you could 1. remove commented transaction block 2. extract this lengthy conditional to separate method? However this would require handling startup_process variable differently. To be honest if startup_process is true most of this method is skipped entirely. Maybe startup_process can live in Lit.startup_process?

@mlitwiniuk mlitwiniuk merged commit 73ab259 into master Nov 3, 2020
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.

3 participants