-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
http: reduce multiple output arrays into one #26004
Conversation
Now we are using `output`, `outputEncodings` and `outputCallbacks` to hold pending data. Reducing them into one array `outputData` can slightly improve performance and reduce some redundant codes.
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. You might get even better performance if you push the data
, encoding
and callback
properties as elements instead of wrapped in an object.
I stress the 'might' because it's not a dead certainty but I'm fairly certain it will. One performance pitfall to be aware of: destructuring assignment from an array (vis-a-vis an object) is quite slow in V8 right now.
I think that's the wrong benchmark to be checking. |
@mscdex I'm now running the whole benchmarks for http module. I'll paste the result after it finishes. |
Here is the whole benchmark result. It seems no difference after change (except
|
lib/_http_outgoing.js
Outdated
socket.cork(); | ||
for (var i = 0; i < outputLength; i++) { | ||
ret = socket.write(output[i], outputEncodings[i], outputCallbacks[i]); | ||
for (var i = 0; i < outputData.length; i++) { |
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.
for (var i = 0; i < outputData.length; i++) { | |
for (var i = 0; i < outputLength; i++) { |
CI before landing: https://ci.nodejs.org/job/node-test-pull-request/20710/ |
Landed in 0b58545 |
Now we are using `output`, `outputEncodings` and `outputCallbacks` to hold pending data. Reducing them into one array `outputData` can slightly improve performance and reduce some redundant codes. PR-URL: #26004 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Now we are using `output`, `outputEncodings` and `outputCallbacks` to hold pending data. Reducing them into one array `outputData` can slightly improve performance and reduce some redundant codes. PR-URL: #26004 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Now we are using
output
,outputEncodings
andoutputCallbacks
to hold pending data. Reducing them into one arrayoutputData
can slightly improve performance and reduce some redundant codes.Here is the benchmark for client request:
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes