Skip to content

Commit

Permalink
lib: revert primordials in a hot path
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aduh95 committed Apr 15, 2021
1 parent 09c9e5d commit b2c7298
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 53 deletions.
3 changes: 1 addition & 2 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const {
MathMin,
Symbol,
RegExpPrototypeTest,
TypedArrayPrototypeSlice,
} = primordials;
const { setImmediate } = require('timers');

Expand Down Expand Up @@ -138,7 +137,7 @@ function parserOnBody(b, start, len) {

// Pretend this was the result of a stream._read call.
if (len > 0 && !stream._dumped) {
const slice = TypedArrayPrototypeSlice(b, start, start + len);
const slice = b.slice(start, start + len);
const ret = stream.push(slice);
if (!ret)
readStop(this.socket);
Expand Down
3 changes: 1 addition & 2 deletions lib/_http_incoming.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

const {
ArrayPrototypePush,
FunctionPrototypeCall,
ObjectDefineProperty,
ObjectSetPrototypeOf,
StringPrototypeCharCodeAt,
Expand Down Expand Up @@ -59,7 +58,7 @@ function IncomingMessage(socket) {
};
}

FunctionPrototypeCall(Readable, this, streamOptions);
Readable.call(this, streamOptions);

this._readableState.readingMore = true;

Expand Down
9 changes: 3 additions & 6 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ const {
ArrayPrototypeJoin,
ArrayPrototypePush,
ArrayPrototypeUnshift,
FunctionPrototype,
FunctionPrototypeBind,
FunctionPrototypeCall,
MathFloor,
NumberPrototypeToString,
ObjectCreate,
Expand Down Expand Up @@ -88,7 +85,7 @@ const { CRLF } = common;

const kCorked = Symbol('corked');

const nop = FunctionPrototype;
const nop = () => {};

const RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i;
const RE_TE_CHUNKED = common.chunkExpression;
Expand All @@ -101,7 +98,7 @@ function isCookieField(s) {
}

function OutgoingMessage() {
FunctionPrototypeCall(Stream, this);
Stream.call(this);

// Queue that holds all currently pending data, until the response will be
// assigned to the socket (until it will its turn in the HTTP pipeline).
Expand Down Expand Up @@ -877,7 +874,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
if (typeof callback === 'function')
this.once('finish', callback);

const finish = FunctionPrototypeBind(onFinish, undefined, this);
const finish = onFinish.bind(undefined, this);

if (this._hasBody && this.chunkedEncoding) {
this._send('0\r\n' + this._trailer + '\r\n', 'latin1', finish);
Expand Down
51 changes: 24 additions & 27 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,13 @@ const {
ArrayPrototypePush,
ArrayPrototypeShift,
Error,
FunctionPrototype,
FunctionPrototypeBind,
FunctionPrototypeCall,
ObjectKeys,
ObjectSetPrototypeOf,
ReflectApply,
RegExpPrototypeTest,
Symbol,
SymbolFor,
TypedArrayPrototypeSlice,
} = primordials;

const net = require('net');
Expand Down Expand Up @@ -185,7 +182,7 @@ class HTTPServerAsyncResource {
}

function ServerResponse(req) {
FunctionPrototypeCall(OutgoingMessage, this);
OutgoingMessage.call(this);

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

Expand Down Expand Up @@ -386,7 +383,7 @@ function Server(options, requestListener) {
validateBoolean(insecureHTTPParser, 'options.insecureHTTPParser');
this.insecureHTTPParser = insecureHTTPParser;

FunctionPrototypeCall(net.Server, this, { allowHalfOpen: true });
net.Server.call(this, { allowHalfOpen: true });

if (requestListener) {
this.on('request', requestListener);
Expand Down Expand Up @@ -493,20 +490,20 @@ function connectionListenerInternal(server, socket) {
outgoingData: 0,
keepAliveTimeoutSet: false
};
state.onData = FunctionPrototypeBind(socketOnData, undefined,
server, socket, parser, state);
state.onEnd = FunctionPrototypeBind(socketOnEnd, undefined,
server, socket, parser, state);
state.onClose = FunctionPrototypeBind(socketOnClose, undefined,
socket, state);
state.onDrain = FunctionPrototypeBind(socketOnDrain, undefined,
socket, state);
state.onData = socketOnData.bind(undefined,
server, socket, parser, state);
state.onEnd = socketOnEnd.bind(undefined,
server, socket, parser, state);
state.onClose = socketOnClose.bind(undefined,
socket, state);
state.onDrain = socketOnDrain.bind(undefined,
socket, state);
socket.on('data', state.onData);
socket.on('error', socketOnError);
socket.on('end', state.onEnd);
socket.on('close', state.onClose);
socket.on('drain', state.onDrain);
parser.onIncoming = FunctionPrototypeBind(parserOnIncoming, undefined,
parser.onIncoming = parserOnIncoming.bind(undefined,
server, socket, state);

// We are consuming socket, so it won't get any actual data
Expand All @@ -527,18 +524,18 @@ function connectionListenerInternal(server, socket) {
parser.consume(socket._handle);
}
parser[kOnExecute] =
FunctionPrototypeBind(onParserExecute, undefined,
server, socket, parser, state);
onParserExecute.bind(undefined,
server, socket, parser, state);

parser[kOnTimeout] =
FunctionPrototypeBind(onParserTimeout, undefined,
server, socket);
onParserTimeout.bind(undefined,
server, socket);

// When receiving new requests on the same socket (pipelining or keep alive)
// make sure the requestTimeout is active.
parser[kOnMessageBegin] =
FunctionPrototypeBind(setRequestTimeout, undefined,
server, socket);
setRequestTimeout.bind(undefined,
server, socket);

// This protects from DOS attack where an attacker establish the connection
// without sending any data on applications where server.timeout is left to
Expand Down Expand Up @@ -649,7 +646,7 @@ function onParserTimeout(server, socket) {
socket.destroy();
}

const noop = FunctionPrototype;
const noop = () => {};
const badRequestResponse = Buffer.from(
`HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}` +
`Connection: close${CRLF}${CRLF}`, 'ascii'
Expand Down Expand Up @@ -719,7 +716,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
const eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
if (eventName === 'upgrade' || server.listenerCount(eventName) > 0) {
debug('SERVER have listener for %s', eventName);
const bodyHead = TypedArrayPrototypeSlice(d, ret, d.length);
const bodyHead = d.slice(ret, d.length);

socket.readableFlowing = null;

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

if (socket._paused && socket.parser) {
Expand Down Expand Up @@ -879,8 +876,8 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {

const res = new server[kServerResponse](req);
res._keepAliveTimeout = server.keepAliveTimeout;
res._onPendingData = FunctionPrototypeBind(updateOutgoingData, undefined,
socket, state);
res._onPendingData = updateOutgoingData.bind(undefined,
socket, state);

res.shouldKeepAlive = keepAlive;
DTRACE_HTTP_SERVER_REQUEST(req, socket);
Expand All @@ -904,8 +901,8 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
// When we're finished writing the response, check if this is the last
// response, if so destroy the socket.
res.on('finish',
FunctionPrototypeBind(resOnFinish, undefined,
req, res, socket, state, server));
resOnFinish.bind(undefined,
req, res, socket, state, server));

if (req.headers.expect !== undefined &&
(req.httpVersionMajor === 1 && req.httpVersionMinor === 1)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const kMaxEventTargetListenersWarned =
Symbol('events.maxEventTargetListenersWarned');

function EventEmitter(opts) {
FunctionPrototypeCall(EventEmitter.init, this, opts);
EventEmitter.init.call(this, opts);
}
module.exports = EventEmitter;
module.exports.once = once;
Expand Down
3 changes: 1 addition & 2 deletions lib/internal/streams/end-of-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
'use strict';

const {
FunctionPrototype,
FunctionPrototypeCall,
ReflectApply,
} = primordials;
Expand Down Expand Up @@ -55,7 +54,7 @@ function isWritableFinished(stream) {
return wState.finished || (wState.ended && wState.length === 0);
}

const nop = FunctionPrototype;
const nop = () => {};

function isReadableEnded(stream) {
if (stream.readableEnded) return true;
Expand Down
3 changes: 1 addition & 2 deletions lib/internal/streams/lazy_transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
'use strict';

const {
FunctionPrototypeCall,
ObjectDefineProperties,
ObjectDefineProperty,
ObjectSetPrototypeOf,
Expand All @@ -26,7 +25,7 @@ ObjectSetPrototypeOf(LazyTransform, stream.Transform);

function makeGetter(name) {
return function() {
FunctionPrototypeCall(stream.Transform, this, this._options);
stream.Transform.call(this, this._options);
this._writableState.decodeStrings = false;

if (!this._options || !this._options.defaultEncoding) {
Expand Down
3 changes: 1 addition & 2 deletions lib/internal/streams/legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@
const {
ArrayIsArray,
ArrayPrototypeUnshift,
FunctionPrototypeCall,
ObjectSetPrototypeOf,
} = primordials;

const EE = require('events');

function Stream(opts) {
FunctionPrototypeCall(EE, this, opts);
EE.call(this, opts);
}
ObjectSetPrototypeOf(Stream.prototype, EE.prototype);
ObjectSetPrototypeOf(Stream, EE);
Expand Down
3 changes: 1 addition & 2 deletions lib/internal/streams/passthrough.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
'use strict';

const {
FunctionPrototypeCall,
ObjectSetPrototypeOf,
} = primordials;

Expand All @@ -40,7 +39,7 @@ function PassThrough(options) {
if (!(this instanceof PassThrough))
return new PassThrough(options);

FunctionPrototypeCall(Transform, this, options);
Transform.call(this, options);
}

PassThrough.prototype._transform = function(chunk, encoding, cb) {
Expand Down
5 changes: 2 additions & 3 deletions lib/internal/streams/readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const {
ArrayPrototypeIndexOf,
ArrayPrototypePush,
ArrayPrototypeSplice,
FunctionPrototype,
FunctionPrototypeBind,
FunctionPrototypeCall,
NumberIsInteger,
Expand Down Expand Up @@ -78,7 +77,7 @@ let from;

ObjectSetPrototypeOf(Readable.prototype, Stream.prototype);
ObjectSetPrototypeOf(Readable, Stream);
const nop = FunctionPrototype;
const nop = () => {};

const { errorOrDestroy } = destroyImpl;

Expand Down Expand Up @@ -208,7 +207,7 @@ function Readable(options) {
addAbortSignalNoValidate(options.signal, this);
}

FunctionPrototypeCall(Stream, this, options);
Stream.call(this, options);

destroyImpl.construct(this, () => {
if (this._readableState.needReadable) {
Expand Down
7 changes: 3 additions & 4 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const {
ArrayPrototypeSplice,
Boolean,
Error,
FunctionPrototype,
FunctionPrototypeCall,
Number,
NumberIsNaN,
Expand Down Expand Up @@ -132,7 +131,7 @@ const DEFAULT_IPV6_ADDR = '::';

const isWindows = process.platform === 'win32';

const noop = FunctionPrototype;
const noop = () => {};

function getFlags(ipv6Only) {
return ipv6Only === true ? TCPConstants.UV_TCP_IPV6ONLY : 0;
Expand Down Expand Up @@ -305,7 +304,7 @@ function Socket(options) {
options.autoDestroy = true;
// Handle strings directly.
options.decodeStrings = false;
ReflectApply(stream.Duplex, this, [options]);
stream.Duplex.call(this, options);

if (options.handle) {
this._handle = options.handle; // private
Expand Down Expand Up @@ -1169,7 +1168,7 @@ function Server(options, connectionListener) {
if (!(this instanceof Server))
return new Server(options, connectionListener);

FunctionPrototypeCall(EventEmitter, this);
EventEmitter.call(this);

if (typeof options === 'function') {
connectionListener = options;
Expand Down

0 comments on commit b2c7298

Please sign in to comment.