-
Notifications
You must be signed in to change notification settings - Fork 253
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
perf(NODE-6356): Improve serialization performance #709
Conversation
98dec41
to
c536d3d
Compare
Hi @SeanReece just I note I rebased this PR on your branch to get our latest perf benchmark updates. Now we can directly check the results in CI without needing to run the tests manually. |
At the PR's current state the standouts on our benchmarks: Notable winners:
Notable losers:
So across the board this impacts large array serialization dramatically - huge gains everywhere. But the deserialization decreases are perplexing since no deserialization code was touched. Maybe just a red herring. EDIT I just re-ran them and still see |
@durran That's great news! Very odd about the deserialize regression though. I've been struggling getting bson-bench running on my M1 Mac, but I was able to get it to run in docker. I'll do some tests and see if I can reproduce. |
@durran I was able to reproduce the issue with Winners 🎉
Losers: None |
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.
LGTM! nice work :)
Description
This MR improves serialization (and
calculateSize
) performance by up to 20-40% in my tests using MFlix documents.serialize
calculateSize
serialize large doc
Object.prototype.toString.call(x)
seems to be quite expensiveObject.prototype.toString.call(x)
)What is changing?
It may look like a lot of changes, but most of the changes here are just re-ordering the type checks during serialization. There shouldn't be any functionality changes here and all tests are passing.
Is there new documentation needed for these changes?
None
What is the motivation for this change?
Release Highlight
Serialization performance improved.
Optimizations have been implemented with respect to BSON serialization across the board, resulting in up to 20% gains in serialization with a sample of MFlix documents. Many thanks to @SeanReece for the contribution.
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript