-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: outgoing cork #29053
http: outgoing cork #29053
Conversation
961eefa
to
0ddf57b
Compare
Not sure exactly how corking a http2 stream works. Would that cork other responses as well? Or does the multiplexing happen on a lower level? |
92fa16b
to
d54f0a5
Compare
multiplexing happens on the native layer. corking an Http2Stream has no impact on any other Http2Stream instances. |
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, but this needs a rebase
650eeab
to
c650a63
Compare
c650a63
to
796f541
Compare
Sorry, there is a test failure. Right now I can't build node due to:
I'll fix this once I've figured out how to build Node again. |
796f541
to
7290bf1
Compare
rebased |
@Trott failed on compile V8? |
Test failures in test-http-response-cork: NaN !== undefined
at Server.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/alpine-latest-x64/test/parallel/test-http-response-cork.js:15:10) |
7290bf1
to
b5a611f
Compare
Hm, that was a minor bug in |
@nodejs/http This could use more reviews. |
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 but I’d put the _stream_duplex.js
changes in a separate commit (while landing)
PR-URL: #29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
Implements cork/uncork on
OutgoingMessage
to make it more streamlike.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes