-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Benchmarking loading JSON file vs YML file #2897
Benchmarking loading JSON file vs YML file #2897
Conversation
…ethod that i18n is using
Just wanted to say that this is in my todo, will get to it soon. Thanks for the PR description! |
Hey @salochara , sorry for the late reply! This is pretty exciting, thanks for doing the investigation. It looks very promising. I'll play around with this idea and get back to you soon. I've been thinking about how we could approach this, and the potential ways are: There are pros and cons to both approaches, so I have to think about it. |
I have written some details on how I made the Haskell fakedata (which uses the same yaml file) faster here: https://psibi.in/posts/2019-09-23-fakedata-perf.html IIRC, the overhead involved was in parsing the yaml file and the IO overhead of reading yaml file was negligible. |
Hello! 👋 Let me know what are your thought on this PR, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing the investigation, this is very cool!
Let's move the json file and use it as a test fixture for now.
Thanks for your review @thdaraujo |
…re/add-benchmark-comparison-json-yml
@thdaraujo I've addressed your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thank you! The performance improvement is quite impressive.
Hi @thdaraujo Just wondering if you're thinking on moving forward with this? Let me know what you think. |
Hey @salochara , sorry for the late reply! Do you want to investigate how much work it would take to convert all the yaml files to json? If we have that, I think we could potentially package the gem without the yaml files and only keep the json files, or set up i18n to only load json files. I haven't investigated this idea yet, just throwing it out there to see what you think. Thanks! |
Motivation / Background
Continues with #2851
This Pull Request has been created because we are comparing loading times between JSON and YML files, as suggested by @thdaraujo.
It turns out that i18n already supports JSON files!
Here's a screenshot on the execution times.
While loading with YML file, took 0.320253 seconds, loading the translation with JSON only took 0.019719 seconds. Meaning, it is waay faster, approximately ~%175 percent faster!
Additional information
Also, I did some testing on my machine and ran the
TestEsMxLocale
test by loading the YML file and then ran it again with the JSON file, and the execution time is significantly faster when passing a JSON.As a note, the reasoning for choosing
YAML.load_file()
andJSON.load_file()
methods in the task, is because those are the methods that i18n is using.load_yaml method: https://github.com/ruby-i18n/i18n/blob/d1762c6c09e50cb1ed32cf2b4f0f525a0531094b/lib/i18n/backend/base.rb#L253
load_json method: https://github.com/ruby-i18n/i18n/blob/d1762c6c09e50cb1ed32cf2b4f0f525a0531094b/lib/i18n/backend/base.rb#L271
I believe this path can lead us to make faker much much faster!
Please, let me know what you guys think! @thdaraujo @stefannibrasil
All the best!
Salo.
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]