Skip to content

Commit ee9e2a2

Browse files
aduh95mcollina
authored andcommitted
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. PR-URL: #38248 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent 4e9212b commit ee9e2a2

20 files changed

+182
-222
lines changed

‎lib/_http_common.js‎

Lines changed: 2 additions & 4 deletions
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-
constslice=TypedArrayPrototypeSlice(b,start,start+len);
139+
constslice=b.slice(start,start+len);
142140
constret=stream.push(slice);
143141
if(!ret)
144142
readStop(this.socket);

‎lib/_http_incoming.js‎

Lines changed: 2 additions & 4 deletions
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
}elseif(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‎

Lines changed: 9 additions & 14 deletions
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
constkCorked=Symbol('corked');
9084

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

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

10397
functionOutgoingMessage(){
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
constheader=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
returnconn.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
returnthis.outputSize<HIGH_WATER_MARK;
@@ -397,9 +391,10 @@ function _storeHeader(firstLine, headers){
397391
}
398392
}elseif(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(leti=0;i<headers.length;i++){
395+
constentry=headers[i];
396+
processHeader(this,state,entry[0],entry[1],true);
397+
}
403398
}else{
404399
if(headers.length%2!==0){
405400
thrownewERR_INVALID_ARG_VALUE('headers',headers);
@@ -877,7 +872,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback){
877872
if(typeofcallback==='function')
878873
this.once('finish',callback);
879874

880-
constfinish=FunctionPrototypeBind(onFinish,undefined,this);
875+
constfinish=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‎

Lines changed: 41 additions & 47 deletions
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
constnet=require('net');
@@ -185,7 +177,7 @@ class HTTPServerAsyncResource{
185177
}
186178

187179
functionServerResponse(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+
constnames=res.getHeaderNames();
418+
for(leti=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
functionabortIncoming(incoming){
596590
while(incoming.length){
597-
constreq=ArrayPrototypeShift(incoming);
591+
constreq=incoming.shift();
598592
req.destroy(connResetException('aborted'));
599593
}
600594
// Abort socket._httpMessage ?
@@ -606,7 +600,7 @@ function socketOnEnd(server, socket, parser, state){
606600
if(retinstanceofError){
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
}elseif(!server.httpAllowHalfOpen){
611605
socket.end();
612606
}elseif(state.outgoing.length){
@@ -629,7 +623,7 @@ function socketOnData(server, socket, parser, state, d){
629623
functiononRequestTimeout(socket){
630624
socket[kRequestTimeout]=undefined;
631625
// socketOnError has additional logic and will call socket.destroy(err).
632-
ReflectApply(socketOnError,socket,[newERR_HTTP_REQUEST_TIMEOUT()]);
626+
socketOnError.call(socket,newERR_HTTP_REQUEST_TIMEOUT());
633627
}
634628

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

652-
constnoop=FunctionPrototype;
646+
constnoop=()=>{};
653647
constbadRequestResponse=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
}elseif(parser.incoming&&parser.incoming.upgrade){
701695
// Upgrade or CONNECT
702696
constreq=parser.incoming;
@@ -719,7 +713,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d){
719713
consteventName=req.method==='CONNECT' ? 'connect' : 'upgrade';
720714
if(eventName==='upgrade'||server.listenerCount(eventName)>0){
721715
debug('SERVER have listener for %s',eventName);
722-
constbodyHead=TypedArrayPrototypeSlice(d,ret,d.length);
716+
constbodyHead=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-
constm=ArrayPrototypeShift(state.outgoing);
832+
constm=state.outgoing.shift();
839833
if(m){
840834
m.assignSocket(socket);
841835
}
@@ -861,7 +855,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive){
861855
return2;
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
constres=newserver[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
functiongenerateSocketListenerWrapper(originalFnName){
979973
returnfunctionsocketListenerWrap(ev,fn){
980-
constres=ReflectApply(net.Socket.prototype[originalFnName],this,
981-
[ev,fn]);
974+
constres=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)