Skip to content

Commit b22a04b

Browse files
mscdexaddaleax
authored andcommitted
http: always cork outgoing writes
PR-URL: #13522 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
1 parent 2122e2f commit b22a04b

File tree

3 files changed

+38
-40
lines changed

3 files changed

+38
-40
lines changed

benchmark/fixtures/simple-http-server.js

+29-33
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@ module.exports = http.createServer(function(req, res) {
2727
dom.add(res);
2828
}
2929

30-
// URL format: /<type>/<length>/<chunks>/<responseBehavior>
30+
// URL format: /<type>/<length>/<chunks>/<responseBehavior>/chunkedEnc
3131
var params = req.url.split('/');
3232
var command = params[1];
3333
var body = '';
3434
var arg = params[2];
3535
var n_chunks = parseInt(params[3], 10);
3636
var resHow = (params.length >= 5 ? params[4] : 'normal');
37+
var chunkedEnc = (params.length >= 6 && params[5] === 'false' ? false : true);
3738
var status = 200;
3839

3940
var n, i;
@@ -95,48 +96,43 @@ module.exports = http.createServer(function(req, res) {
9596

9697
// example: http://localhost:port/bytes/512/4
9798
// sends a 512 byte body in 4 chunks of 128 bytes
98-
if (n_chunks > 0) {
99-
switch (resHow) {
100-
case 'setHeader':
101-
res.statusCode = status;
102-
res.setHeader('Content-Type', 'text/plain');
99+
var len = body.length;
100+
switch (resHow) {
101+
case 'setHeader':
102+
res.statusCode = status;
103+
res.setHeader('Content-Type', 'text/plain');
104+
if (chunkedEnc)
103105
res.setHeader('Transfer-Encoding', 'chunked');
104-
break;
105-
case 'setHeaderWH':
106-
res.setHeader('Content-Type', 'text/plain');
106+
else
107+
res.setHeader('Content-Length', len.toString());
108+
break;
109+
case 'setHeaderWH':
110+
res.setHeader('Content-Type', 'text/plain');
111+
if (chunkedEnc)
107112
res.writeHead(status, { 'Transfer-Encoding': 'chunked' });
108-
break;
109-
default:
113+
else
114+
res.writeHead(status, { 'Content-Length': len.toString() });
115+
break;
116+
default:
117+
if (chunkedEnc) {
110118
res.writeHead(status, {
111119
'Content-Type': 'text/plain',
112120
'Transfer-Encoding': 'chunked'
113121
});
114-
}
115-
// send body in chunks
116-
var len = body.length;
122+
} else {
123+
res.writeHead(status, {
124+
'Content-Type': 'text/plain',
125+
'Content-Length': len.toString()
126+
});
127+
}
128+
}
129+
// send body in chunks
130+
if (n_chunks > 1) {
117131
var step = Math.floor(len / n_chunks) || 1;
118-
119-
for (i = 0, n = (n_chunks - 1); i < n; ++i) {
132+
for (i = 0, n = (n_chunks - 1); i < n; ++i)
120133
res.write(body.slice(i * step, i * step + step));
121-
}
122134
res.end(body.slice((n_chunks - 1) * step));
123135
} else {
124-
switch (resHow) {
125-
case 'setHeader':
126-
res.statusCode = status;
127-
res.setHeader('Content-Type', 'text/plain');
128-
res.setHeader('Content-Length', body.length.toString());
129-
break;
130-
case 'setHeaderWH':
131-
res.setHeader('Content-Type', 'text/plain');
132-
res.writeHead(status, { 'Content-Length': body.length.toString() });
133-
break;
134-
default:
135-
res.writeHead(status, {
136-
'Content-Type': 'text/plain',
137-
'Content-Length': body.length.toString()
138-
});
139-
}
140136
res.end(body);
141137
}
142138
});

benchmark/http/simple.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ var bench = common.createBenchmark(main, {
66
// unicode confuses ab on os x.
77
type: ['bytes', 'buffer'],
88
len: [4, 1024, 102400],
9-
chunks: [0, 1, 4], // chunks=0 means 'no chunked encoding'.
9+
chunks: [1, 4],
1010
c: [50, 500],
11+
chunkedEnc: ['true', 'false'],
1112
res: ['normal', 'setHeader', 'setHeaderWH']
1213
});
1314

@@ -16,7 +17,8 @@ function main(conf) {
1617
var server = require('../fixtures/simple-http-server.js')
1718
.listen(process.env.PORT || common.PORT)
1819
.on('listening', function() {
19-
var path = `/${conf.type}/${conf.len}/${conf.chunks}/${conf.res}`;
20+
var path =
21+
`/${conf.type}/${conf.len}/${conf.chunks}/${conf.res}/${conf.chunkedEnc}`;
2022

2123
bench.http({
2224
path: path,

lib/_http_outgoing.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -660,18 +660,18 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
660660
// signal the user to keep writing.
661661
if (chunk.length === 0) return true;
662662

663+
if (!fromEnd && msg.connection && !msg.connection.corked) {
664+
msg.connection.cork();
665+
process.nextTick(connectionCorkNT, msg.connection);
666+
}
667+
663668
var len, ret;
664669
if (msg.chunkedEncoding) {
665670
if (typeof chunk === 'string')
666671
len = Buffer.byteLength(chunk, encoding);
667672
else
668673
len = chunk.length;
669674

670-
if (msg.connection && !msg.connection.corked) {
671-
msg.connection.cork();
672-
process.nextTick(connectionCorkNT, msg.connection);
673-
}
674-
675675
msg._send(len.toString(16), 'latin1', null);
676676
msg._send(crlf_buf, null, null);
677677
msg._send(chunk, encoding, null);

0 commit comments

Comments
 (0)