Skip to content

Performance: Cache translations to reduce file read and parse operations #2939

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.

Sign up for GitHub

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

Open
alextaujenis wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Performance: Cache translations to reduce file read and parse operations #2939

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
756767a
Cache translations to reduce file read and parse operations
alextaujenis Apr 19, 2024
379b0b3
Disable rubocop Style/ClassVars
alextaujenis Apr 19, 2024
7233dfa
Enable rubocop Style/ClassVars
alextaujenis Apr 19, 2024
dac7cff
Merge branch 'faker-ruby:main' into translate-performance
alextaujenis Apr 22, 2024
aa82207
Merge branch 'main' into translate-performance
alextaujenis May 8, 2024
559859a
Convert the translate cache to a thread safe variable
alextaujenis May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/faker.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def random
end
end

# rubocop:disable Style/ClassVars
class Base
Numbers = Array(0..9)
ULetters = Array('A'..'Z')
Expand Down Expand Up @@ -157,12 +158,17 @@ def parse(key)
parts.any? ? parts.join : numerify(fetched)
end

def translate_cache
Thread.current[:faker_translate_cache] ||= {}
end

# Call I18n.translate with our configured locale if no
# locale is specified
def translate(*args, **opts)
opts[:locale] ||= Faker::Config.locale
translate_key = args.to_s + opts.sort.join
Copy link
Contributor

@thdaraujo thdaraujo May 6, 2024

Choose a reason for hiding this comment

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

maybe translate_key should be hashed?

Copy link
Contributor Author

@alextaujenis alextaujenis May 8, 2024

Choose a reason for hiding this comment

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

It does sound reasonable to hash the translate_key, but there is no reason to compute this digest because the translate_key (as provided in this PR) is a valid string and can be used for lookups. The extra time taken on every method call to compute even an md5 digest erases all performance gains for this PR.

Another interesting fact here is that i18n computes the sha256 digest for all cache keys if you enable their default translation cache. This suggests that implementing the i18n default cache will also erase the performance gains for this PR.

opts[:raise] = true
I18n.translate(*args, **opts)
translate_cache[translate_key] ||= I18n.translate(*args, **opts)
rescue I18n::MissingTranslationData
opts[:locale] = :en

Expand Down Expand Up @@ -268,6 +274,7 @@ def disable_enforce_available_locales
end
end
end
# rubocop:enable Style/ClassVars
end

# require faker objects
Expand Down