-
Notifications
You must be signed in to change notification settings - Fork 131
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
JSON lockfile #1280
JSON lockfile #1280
Conversation
@f-f it's a definite improvement! The decoding and parsing is a bit more coupled now so I didn't bother check how long each takes individually but the combined decode+parse took 2.319s on this branch and 3.106s on master 🎉 Given the yaml parsing took 639.543ms before, I'm guessing the parsing is now practically instant and the decoding is roughly the same (would make sense). |
Right, so I think this branch is worth merging. I'll fix the tests. @finnhodgkin would you be able to share the giant lockfile somewhere so I can look at it? My email is in my GitHub profile if you'd rather not share it publicly. @garyb would you have any input on what we could do to improve JSON decoding here? |
@f-f I've emailed it 👍 |
@finnhodgkin thanks! 3MB of lockfile, most of which is |
Not really unfortunately - from looking through the codecs a bit I don't see anything out of the ordinary. The codec library definitely has some overhead, but I don't know the specific bottleneck(s) that would contribute to this, I'd have to have a poke around in a profiler to be able to say anything specific. At a guess I would imagine record codecs are the biggest culprit though, and I'm not really sure how to squeeze much improvement out of them due to the nature of the machinery behind them. Writing them "manually" might be the only way to get really good performance. |
This is an attempt to improve the performance of lockfile parsing, which is currently super slow (see #1262), by changing the format from YAML to JSON.
We don't have benchmarks in CI, but my local testing is showing that the time spent parsing the lockfile for this project went from ~150ms to ~100ms.
Our lockfile is pretty small so I hope there are bigger gains for bigger projects - @finnhodgkin could you try this out on your project and see if the performance is any better?