Skip to content

Commit 680ecc4

Browse files
committed
lib: revert primordials in a hot path
Evidence has shown that use of primordials have sometimes an impact of performance. This commit reverts the changes who are most likely to be responsible for performance regression in the HTTP response path.
1 parent bc31dc0 commit 680ecc4

20 files changed

+182
-222
lines changed

lib/_http_common.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,9 @@
2222
'use strict';
2323

2424
const {
25-
ArrayPrototypePushApply,
2625
MathMin,
2726
Symbol,
2827
RegExpPrototypeTest,
29-
TypedArrayPrototypeSlice,
3028
} = primordials;
3129
const { setImmediate } = require('timers');
3230

@@ -66,7 +64,7 @@ function parserOnHeaders(headers, url) {
6664
// Once we exceeded headers limit - stop collecting them
6765
if (this.maxHeaderPairs <= 0 ||
6866
this._headers.length < this.maxHeaderPairs) {
69-
ArrayPrototypePushApply(this._headers, headers);
67+
this._headers.push(...headers);
7068
}
7169
this._url += url;
7270
}
@@ -138,7 +136,7 @@ function parserOnBody(b, start, len) {
138136

139137
// Pretend this was the result of a stream._read call.
140138
if (len > 0 && !stream._dumped) {
141-
const slice = TypedArrayPrototypeSlice(b, start, start + len);
139+
const slice = b.slice(start, start + len);
142140
const ret = stream.push(slice);
143141
if (!ret)
144142
readStop(this.socket);

lib/_http_incoming.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
'use strict';
2323

2424
const {
25-
ArrayPrototypePush,
26-
FunctionPrototypeCall,
2725
ObjectDefineProperty,
2826
ObjectSetPrototypeOf,
2927
StringPrototypeCharCodeAt,
@@ -59,7 +57,7 @@ function IncomingMessage(socket) {
5957
};
6058
}
6159

62-
FunctionPrototypeCall(Readable, this, streamOptions);
60+
Readable.call(this, streamOptions);
6361

6462
this._readableState.readingMore = true;
6563

@@ -350,7 +348,7 @@ function _addHeaderLine(field, value, dest) {
350348
} else if (flag === 1) {
351349
// Array header -- only Set-Cookie at the moment
352350
if (dest['set-cookie'] !== undefined) {
353-
ArrayPrototypePush(dest['set-cookie'], value);
351+
dest['set-cookie'].push(value);
354352
} else {
355353
dest['set-cookie'] = [value];
356354
}

lib/_http_outgoing.js

+9-14
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,7 @@
2424
const {
2525
Array,
2626
ArrayIsArray,
27-
ArrayPrototypeForEach,
2827
ArrayPrototypeJoin,
29-
ArrayPrototypePush,
30-
ArrayPrototypeUnshift,
31-
FunctionPrototype,
32-
FunctionPrototypeBind,
33-
FunctionPrototypeCall,
3428
MathFloor,
3529
NumberPrototypeToString,
3630
ObjectCreate,
@@ -88,7 +82,7 @@ const { CRLF } = common;
8882

8983
const kCorked = Symbol('corked');
9084

91-
const nop = FunctionPrototype;
85+
const nop = () => {};
9286

9387
const RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i;
9488
const RE_TE_CHUNKED = common.chunkExpression;
@@ -101,7 +95,7 @@ function isCookieField(s) {
10195
}
10296

10397
function OutgoingMessage() {
104-
FunctionPrototypeCall(Stream, this);
98+
Stream.call(this);
10599

106100
// Queue that holds all currently pending data, until the response will be
107101
// assigned to the socket (until it will its turn in the HTTP pipeline).
@@ -331,7 +325,7 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
331325
data = this._header + data;
332326
} else {
333327
const header = this._header;
334-
ArrayPrototypeUnshift(this.outputData, {
328+
this.outputData.unshift({
335329
data: header,
336330
encoding: 'latin1',
337331
callback: null
@@ -368,7 +362,7 @@ function _writeRaw(data, encoding, callback) {
368362
return conn.write(data, encoding, callback);
369363
}
370364
// Buffer, as long as we're not destroyed.
371-
ArrayPrototypePush(this.outputData, { data, encoding, callback });
365+
this.outputData.push({ data, encoding, callback });
372366
this.outputSize += data.length;
373367
this._onPendingData(data.length);
374368
return this.outputSize < HIGH_WATER_MARK;
@@ -397,9 +391,10 @@ function _storeHeader(firstLine, headers) {
397391
}
398392
} else if (ArrayIsArray(headers)) {
399393
if (headers.length && ArrayIsArray(headers[0])) {
400-
ArrayPrototypeForEach(headers, (entry) =>
401-
processHeader(this, state, entry[0], entry[1], true)
402-
);
394+
for (let i = 0; i < headers.length; i++) {
395+
const entry = headers[i];
396+
processHeader(this, state, entry[0], entry[1], true);
397+
}
403398
} else {
404399
if (headers.length % 2 !== 0) {
405400
throw new ERR_INVALID_ARG_VALUE('headers', headers);
@@ -877,7 +872,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
877872
if (typeof callback === 'function')
878873
this.once('finish', callback);
879874

880-
const finish = FunctionPrototypeBind(onFinish, undefined, this);
875+
const finish = onFinish.bind(undefined, this);
881876

882877
if (this._hasBody && this.chunkedEncoding) {
883878
this._send('0\r\n' + this._trailer + '\r\n', 'latin1', finish);

lib/_http_server.js

+41-47
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,12 @@
2323

2424
const {
2525
ArrayIsArray,
26-
ArrayPrototypeForEach,
27-
ArrayPrototypePush,
28-
ArrayPrototypeShift,
2926
Error,
30-
FunctionPrototype,
31-
FunctionPrototypeBind,
32-
FunctionPrototypeCall,
3327
ObjectKeys,
3428
ObjectSetPrototypeOf,
35-
ReflectApply,
3629
RegExpPrototypeTest,
3730
Symbol,
3831
SymbolFor,
39-
TypedArrayPrototypeSlice,
4032
} = primordials;
4133

4234
const net = require('net');
@@ -185,7 +177,7 @@ class HTTPServerAsyncResource {
185177
}
186178

187179
function ServerResponse(req) {
188-
FunctionPrototypeCall(OutgoingMessage, this);
180+
OutgoingMessage.call(this);
189181

190182
if (req.method === 'HEAD') this._hasBody = false;
191183

@@ -212,7 +204,7 @@ ObjectSetPrototypeOf(ServerResponse, OutgoingMessage);
212204
ServerResponse.prototype._finish = function _finish() {
213205
DTRACE_HTTP_SERVER_RESPONSE(this.socket);
214206
emitStatistics(this[kServerResponseStatistics]);
215-
FunctionPrototypeCall(OutgoingMessage.prototype._finish, this);
207+
OutgoingMessage.prototype._finish.call(this);
216208
};
217209

218210

@@ -386,7 +378,7 @@ function Server(options, requestListener) {
386378
validateBoolean(insecureHTTPParser, 'options.insecureHTTPParser');
387379
this.insecureHTTPParser = insecureHTTPParser;
388380

389-
FunctionPrototypeCall(net.Server, this, { allowHalfOpen: true });
381+
net.Server.call(this, { allowHalfOpen: true });
390382

391383
if (requestListener) {
392384
this.on('request', requestListener);
@@ -422,17 +414,19 @@ Server.prototype[EE.captureRejectionSymbol] = function(err, event, ...args) {
422414
const { 1: res } = args;
423415
if (!res.headersSent && !res.writableEnded) {
424416
// Don't leak headers.
425-
ArrayPrototypeForEach(res.getHeaderNames(),
426-
(name) => res.removeHeader(name));
417+
const names = res.getHeaderNames();
418+
for (let i = 0; i < names.length; i++) {
419+
res.removeHeader(names[i]);
420+
}
427421
res.statusCode = 500;
428422
res.end(STATUS_CODES[500]);
429423
} else {
430424
res.destroy();
431425
}
432426
break;
433427
default:
434-
ReflectApply(net.Server.prototype[SymbolFor('nodejs.rejection')],
435-
this, arguments);
428+
net.Server.prototype[SymbolFor('nodejs.rejection')]
429+
.apply(this, arguments);
436430
}
437431
};
438432

@@ -493,20 +487,20 @@ function connectionListenerInternal(server, socket) {
493487
outgoingData: 0,
494488
keepAliveTimeoutSet: false
495489
};
496-
state.onData = FunctionPrototypeBind(socketOnData, undefined,
497-
server, socket, parser, state);
498-
state.onEnd = FunctionPrototypeBind(socketOnEnd, undefined,
499-
server, socket, parser, state);
500-
state.onClose = FunctionPrototypeBind(socketOnClose, undefined,
501-
socket, state);
502-
state.onDrain = FunctionPrototypeBind(socketOnDrain, undefined,
503-
socket, state);
490+
state.onData = socketOnData.bind(undefined,
491+
server, socket, parser, state);
492+
state.onEnd = socketOnEnd.bind(undefined,
493+
server, socket, parser, state);
494+
state.onClose = socketOnClose.bind(undefined,
495+
socket, state);
496+
state.onDrain = socketOnDrain.bind(undefined,
497+
socket, state);
504498
socket.on('data', state.onData);
505499
socket.on('error', socketOnError);
506500
socket.on('end', state.onEnd);
507501
socket.on('close', state.onClose);
508502
socket.on('drain', state.onDrain);
509-
parser.onIncoming = FunctionPrototypeBind(parserOnIncoming, undefined,
503+
parser.onIncoming = parserOnIncoming.bind(undefined,
510504
server, socket, state);
511505

512506
// We are consuming socket, so it won't get any actual data
@@ -527,18 +521,18 @@ function connectionListenerInternal(server, socket) {
527521
parser.consume(socket._handle);
528522
}
529523
parser[kOnExecute] =
530-
FunctionPrototypeBind(onParserExecute, undefined,
531-
server, socket, parser, state);
524+
onParserExecute.bind(undefined,
525+
server, socket, parser, state);
532526

533527
parser[kOnTimeout] =
534-
FunctionPrototypeBind(onParserTimeout, undefined,
535-
server, socket);
528+
onParserTimeout.bind(undefined,
529+
server, socket);
536530

537531
// When receiving new requests on the same socket (pipelining or keep alive)
538532
// make sure the requestTimeout is active.
539533
parser[kOnMessageBegin] =
540-
FunctionPrototypeBind(setRequestTimeout, undefined,
541-
server, socket);
534+
setRequestTimeout.bind(undefined,
535+
server, socket);
542536

543537
// This protects from DOS attack where an attacker establish the connection
544538
// without sending any data on applications where server.timeout is left to
@@ -594,7 +588,7 @@ function socketOnClose(socket, state) {
594588

595589
function abortIncoming(incoming) {
596590
while (incoming.length) {
597-
const req = ArrayPrototypeShift(incoming);
591+
const req = incoming.shift();
598592
req.destroy(connResetException('aborted'));
599593
}
600594
// Abort socket._httpMessage ?
@@ -606,7 +600,7 @@ function socketOnEnd(server, socket, parser, state) {
606600
if (ret instanceof Error) {
607601
debug('parse error');
608602
// socketOnError has additional logic and will call socket.destroy(err).
609-
FunctionPrototypeCall(socketOnError, socket, ret);
603+
socketOnError.call(socket, ret);
610604
} else if (!server.httpAllowHalfOpen) {
611605
socket.end();
612606
} else if (state.outgoing.length) {
@@ -629,7 +623,7 @@ function socketOnData(server, socket, parser, state, d) {
629623
function onRequestTimeout(socket) {
630624
socket[kRequestTimeout] = undefined;
631625
// socketOnError has additional logic and will call socket.destroy(err).
632-
ReflectApply(socketOnError, socket, [new ERR_HTTP_REQUEST_TIMEOUT()]);
626+
socketOnError.call(socket, new ERR_HTTP_REQUEST_TIMEOUT());
633627
}
634628

635629
function onParserExecute(server, socket, parser, state, ret) {
@@ -649,7 +643,7 @@ function onParserTimeout(server, socket) {
649643
socket.destroy();
650644
}
651645

652-
const noop = FunctionPrototype;
646+
const noop = () => {};
653647
const badRequestResponse = Buffer.from(
654648
`HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}` +
655649
`Connection: close${CRLF}${CRLF}`, 'ascii'
@@ -696,7 +690,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
696690
prepareError(ret, parser, d);
697691
ret.rawPacket = d || parser.getCurrentBuffer();
698692
debug('parse error', ret);
699-
FunctionPrototypeCall(socketOnError, socket, ret);
693+
socketOnError.call(socket, ret);
700694
} else if (parser.incoming && parser.incoming.upgrade) {
701695
// Upgrade or CONNECT
702696
const req = parser.incoming;
@@ -719,7 +713,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
719713
const eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
720714
if (eventName === 'upgrade' || server.listenerCount(eventName) > 0) {
721715
debug('SERVER have listener for %s', eventName);
722-
const bodyHead = TypedArrayPrototypeSlice(d, ret, d.length);
716+
const bodyHead = d.slice(ret, d.length);
723717

724718
socket.readableFlowing = null;
725719

@@ -738,7 +732,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
738732
// When receiving new requests on the same socket (pipelining or keep alive)
739733
// make sure the requestTimeout is active.
740734
parser[kOnMessageBegin] =
741-
FunctionPrototypeBind(setRequestTimeout, undefined, server, socket);
735+
setRequestTimeout.bind(undefined, server, socket);
742736
}
743737

744738
if (socket._paused && socket.parser) {
@@ -802,7 +796,7 @@ function resOnFinish(req, res, socket, state, server) {
802796
// array will be empty.
803797
assert(state.incoming.length === 0 || state.incoming[0] === req);
804798

805-
ArrayPrototypeShift(state.incoming);
799+
state.incoming.shift();
806800

807801
// If the user never called req.read(), and didn't pipe() or
808802
// .resume() or .on('data'), then we call req._dump() so that the
@@ -835,7 +829,7 @@ function resOnFinish(req, res, socket, state, server) {
835829
}
836830
} else {
837831
// Start sending the next message
838-
const m = ArrayPrototypeShift(state.outgoing);
832+
const m = state.outgoing.shift();
839833
if (m) {
840834
m.assignSocket(socket);
841835
}
@@ -861,7 +855,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
861855
return 2;
862856
}
863857

864-
ArrayPrototypePush(state.incoming, req);
858+
state.incoming.push(req);
865859

866860
// If the writable end isn't consuming, then stop reading
867861
// so that we don't become overwhelmed by a flood of
@@ -879,8 +873,8 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
879873

880874
const res = new server[kServerResponse](req);
881875
res._keepAliveTimeout = server.keepAliveTimeout;
882-
res._onPendingData = FunctionPrototypeBind(updateOutgoingData, undefined,
883-
socket, state);
876+
res._onPendingData = updateOutgoingData.bind(undefined,
877+
socket, state);
884878

885879
res.shouldKeepAlive = keepAlive;
886880
DTRACE_HTTP_SERVER_REQUEST(req, socket);
@@ -896,16 +890,16 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
896890

897891
if (socket._httpMessage) {
898892
// There are already pending outgoing res, append.
899-
ArrayPrototypePush(state.outgoing, res);
893+
state.outgoing.push(res);
900894
} else {
901895
res.assignSocket(socket);
902896
}
903897

904898
// When we're finished writing the response, check if this is the last
905899
// response, if so destroy the socket.
906900
res.on('finish',
907-
FunctionPrototypeBind(resOnFinish, undefined,
908-
req, res, socket, state, server));
901+
resOnFinish.bind(undefined,
902+
req, res, socket, state, server));
909903

910904
if (req.headers.expect !== undefined &&
911905
(req.httpVersionMajor === 1 && req.httpVersionMinor === 1)) {
@@ -977,8 +971,8 @@ function unconsume(parser, socket) {
977971

978972
function generateSocketListenerWrapper(originalFnName) {
979973
return function socketListenerWrap(ev, fn) {
980-
const res = ReflectApply(net.Socket.prototype[originalFnName], this,
981-
[ev, fn]);
974+
const res = net.Socket.prototype[originalFnName].call(this,
975+
ev, fn);
982976
if (!this.parser) {
983977
this.on = net.Socket.prototype.on;
984978
this.addListener = net.Socket.prototype.addListener;

0 commit comments

Comments
 (0)