-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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-5771): improve new connection #3948
Conversation
70d6ef8
to
bb83edb
Compare
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.
nothing blocking since I'm OOO starting tomorrow, just some 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.
One question - did we also profile memory consumption?
Hey @nbbeeken @durran - not sure how to easily bring this to attention, but we discovered this issue: Automattic/mongoose#14877 and it seems to pin down to 6.3.0 is unaffected. There's a repro in the issue. |
Description
What is changing?
abortable
helper to not create a "done" signal, instead simplify the promise wrapping and abort listener removal to lower performance costIs there new documentation needed for these changes?
No.
What is the motivation for this change?
Improves the performance of the new connection layer to be on par with our existing connection layer. The major cost was constructing many AbortSignals, however, going back to using a stream to just collect chunks gets us within a margin of error of the old performance.
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript