Skip to content
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.

Sign up for GitHub

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

Jump to bottom

lib: revert primordials in a hot path #38246

Closed
wants to merge 8 commits into from
Closed

lib: revert primordials in a hot path #38246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f49ba49
Revert "stream: refactor to use more primordials"
mcollina Apr 10, 2021
f9763d8
stream: removed number primordials
mcollina Apr 13, 2021
1e0d89c
http: remove primordials from hot path
mcollina Apr 13, 2021
74d2497
lib: revert more primordials in the hot path
mcollina Apr 14, 2021
590a545
fixup
mcollina Apr 14, 2021
dbbdd59
fixhup: less primordials in hot path
mcollina Apr 15, 2021
4553526
fixup
mcollina Apr 15, 2021
e51967c
fixup: remove an always-on primordial in debuglog.
mcollina Apr 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 5 additions & 9 deletions lib/_http_common.js
View file
Open in desktop
Expand Up @@ -22,11 +22,7 @@
'use strict';

const {
ArrayPrototypePushApply,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the ArrayPrototype operations where clearly problematic in my analysis.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that all of them would be, but I guess if that's the case it's OK to remove them all and try to add them back later if V8 optimize this away.

MathMin,
Symbol,
RegExpPrototypeTest,
TypedArrayPrototypeSlice,
} = primordials;
const { setImmediate } = require('timers');

Expand Down Expand Up @@ -66,7 +62,7 @@ function parserOnHeaders(headers, url) {
// Once we exceeded headers limit - stop collecting them
if (this.maxHeaderPairs <= 0 ||
this._headers.length < this.maxHeaderPairs) {
ArrayPrototypePushApply(this._headers, headers);
this._headers = this._headers.concat(headers);
}
this._url += url;
}
Expand Down Expand Up @@ -113,7 +109,7 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,

// If parser.maxHeaderPairs <= 0 assume that there's no limit.
if (parser.maxHeaderPairs > 0)
n = MathMin(n, parser.maxHeaderPairs);
n = Math.min(n, parser.maxHeaderPairs);

incoming._addHeaderLines(headers, n);

Expand All @@ -138,7 +134,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 Expand Up @@ -220,7 +216,7 @@ const tokenRegExp = /^[\^_`a-zA-Z\-0-9!#$%&'*+.|~]+$/;
* See https://tools.ietf.org/html/rfc7230#section-3.2.6
*/
function checkIsHttpToken(val) {
return RegExpPrototypeTest(tokenRegExp, val);
return tokenRegExp.test(val);
}

const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/;
Expand All @@ -231,7 +227,7 @@ const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/;
* field-vchar = VCHAR / obs-text
*/
function checkInvalidHeaderChar(val) {
return RegExpPrototypeTest(headerCharRegex, val);
return headerCharRegex.test(val);
}

function cleanParser(parser) {
Expand Down
15 changes: 5 additions & 10 deletions lib/_http_incoming.js
View file
Open in desktop
Expand Up @@ -22,13 +22,8 @@
'use strict';

const {
ArrayPrototypePush,
FunctionPrototypeCall,
ObjectDefineProperty,
ObjectSetPrototypeOf,
StringPrototypeCharCodeAt,
StringPrototypeSlice,
StringPrototypeToLowerCase,
Symbol
} = primordials;

Expand Down Expand Up @@ -59,7 +54,7 @@ function IncomingMessage(socket) {
};
}

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

this._readableState.readingMore = true;

Expand Down Expand Up @@ -324,7 +319,7 @@ function matchKnownFields(field, lowercased) {
if (lowercased) {
return '\u0000' + field;
}
return matchKnownFields(StringPrototypeToLowerCase(field), true);
return matchKnownFields(field.toLowerCase(), true);
}
// Add the given (field, value) pair to the message
//
Expand All @@ -338,9 +333,9 @@ function matchKnownFields(field, lowercased) {
IncomingMessage.prototype._addHeaderLine = _addHeaderLine;
function _addHeaderLine(field, value, dest) {
field = matchKnownFields(field);
const flag = StringPrototypeCharCodeAt(field, 0);
const flag = field.charCodeAt(0);
if (flag === 0 || flag === 2) {
field = StringPrototypeSlice(field, 1);
field = field.slice(1);
// Make a delimited list
if (typeof dest[field] === 'string') {
dest[field] += (flag === 0 ? ', ' : '; ') + value;
Expand All @@ -350,7 +345,7 @@ function _addHeaderLine(field, value, dest) {
} else if (flag === 1) {
// Array header -- only Set-Cookie at the moment
if (dest['set-cookie'] !== undefined) {
ArrayPrototypePush(dest['set-cookie'], value);
dest['set-cookie'].push(value);
} else {
dest['set-cookie'] = [value];
}
Expand Down
87 changes: 36 additions & 51 deletions lib/_http_outgoing.js
View file
Open in desktop
Expand Up @@ -22,25 +22,9 @@
'use strict';

const {
Array,
ArrayIsArray,
ArrayPrototypeForEach,
ArrayPrototypeJoin,
ArrayPrototypePush,
ArrayPrototypeUnshift,
FunctionPrototype,
FunctionPrototypeBind,
FunctionPrototypeCall,
MathFloor,
NumberPrototypeToString,
ObjectCreate,
ObjectDefineProperty,
ObjectKeys,
ObjectValues,
ObjectPrototypeHasOwnProperty,
ObjectSetPrototypeOf,
RegExpPrototypeTest,
StringPrototypeToLowerCase,
Symbol,
} = primordials;

Expand Down Expand Up @@ -88,7 +72,7 @@ const { CRLF } = common;

const kCorked = Symbol('corked');

const nop = FunctionPrototype;
const nop = function () {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const nop = function () {};
const nop = function() {};


const RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i;
const RE_TE_CHUNKED = common.chunkExpression;
Expand All @@ -97,11 +81,11 @@ const RE_TE_CHUNKED = common.chunkExpression;
// against the word "cookie." As of V8 6.6 this is faster than handrolling or
// using a case-insensitive RegExp.
function isCookieField(s) {
return s.length === 6 && StringPrototypeToLowerCase(s) === 'cookie';
return s.length === 6 && s.toLowerCase() === 'cookie';
}

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 @@ -190,13 +174,13 @@ ObjectDefineProperty(OutgoingMessage.prototype, '_headers', {
if (val == null) {
this[kOutHeaders] = null;
} else if (typeof val === 'object') {
const headers = this[kOutHeaders] = ObjectCreate(null);
const keys = ObjectKeys(val);
const headers = this[kOutHeaders] = Object.create(null);
const keys = Object.keys(val);
// Retain for(;;) loop for performance reasons
// Refs: https://github.com/nodejs/node/pull/30958
for (let i = 0; i < keys.length; ++i) {
const name = keys[i];
headers[StringPrototypeToLowerCase(name)] = [name, val[name]];
headers[name.toLowerCase()] = [name, val[name]];
}
}
}, 'OutgoingMessage.prototype._headers is deprecated', 'DEP0066')
Expand All @@ -215,8 +199,8 @@ ObjectDefineProperty(OutgoingMessage.prototype, '_headerNames', {
get: internalUtil.deprecate(function() {
const headers = this[kOutHeaders];
if (headers !== null) {
const out = ObjectCreate(null);
const keys = ObjectKeys(headers);
const out = Object.create(null);
const keys = Object.keys(headers);
// Retain for(;;) loop for performance reasons
// Refs: https://github.com/nodejs/node/pull/30958
for (let i = 0; i < keys.length; ++i) {
Expand All @@ -233,7 +217,7 @@ ObjectDefineProperty(OutgoingMessage.prototype, '_headerNames', {
const headers = this[kOutHeaders];
if (!headers)
return;
const keys = ObjectKeys(val);
const keys = Object.keys(val);
// Retain for(;;) loop for performance reasons
// Refs: https://github.com/nodejs/node/pull/30958
for (let i = 0; i < keys.length; ++i) {
Expand All @@ -255,7 +239,7 @@ OutgoingMessage.prototype._renderHeaders = function _renderHeaders() {
const headers = {};

if (headersMap !== null) {
const keys = ObjectKeys(headersMap);
const keys = Object.keys(headersMap);
// Retain for(;;) loop for performance reasons
// Refs: https://github.com/nodejs/node/pull/30958
for (let i = 0, l = keys.length; i < l; i++) {
Expand Down Expand Up @@ -331,7 +315,7 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
data = this._header + data;
} else {
const header = this._header;
ArrayPrototypeUnshift(this.outputData, {
this.outputData.unshift({
data: header,
encoding: 'latin1',
callback: null
Expand Down Expand Up @@ -368,7 +352,7 @@ function _writeRaw(data, encoding, callback) {
return conn.write(data, encoding, callback);
}
// Buffer, as long as we're not destroyed.
ArrayPrototypePush(this.outputData, { data, encoding, callback });
this.outputData.push({ data, encoding, callback });
this.outputSize += data.length;
this._onPendingData(data.length);
return this.outputSize < HIGH_WATER_MARK;
Expand All @@ -395,11 +379,12 @@ function _storeHeader(firstLine, headers) {
const entry = headers[key];
processHeader(this, state, entry[0], entry[1], false);
}
} else if (ArrayIsArray(headers)) {
if (headers.length && ArrayIsArray(headers[0])) {
ArrayPrototypeForEach(headers, (entry) =>
} else if (Array.isArray(headers)) {
if (headers.length && Array.isArray(headers[0])) {
for (let n = 0; n < headers.length; n++) {
const entry = headers[n]
processHeader(this, state, entry[0], entry[1], true)
);
}
} else {
if (headers.length % 2 !== 0) {
throw new ERR_INVALID_ARG_VALUE('headers', headers);
Expand Down Expand Up @@ -454,7 +439,7 @@ function _storeHeader(firstLine, headers) {
if (shouldSendKeepAlive) {
header += 'Connection: keep-alive' + CRLF;
if (this._keepAliveTimeout && this._defaultKeepAlive) {
const timeoutSeconds = MathFloor(this._keepAliveTimeout / 1000);
const timeoutSeconds = Math.floor(this._keepAliveTimeout / 1000);
header += `Keep-Alive: timeout=${timeoutSeconds}${CRLF}`;
}
} else {
Expand Down Expand Up @@ -504,15 +489,15 @@ function _storeHeader(firstLine, headers) {
function processHeader(self, state, key, value, validate) {
if (validate)
validateHeaderName(key);
if (ArrayIsArray(value)) {
if (Array.isArray(value)) {
if (value.length < 2 || !isCookieField(key)) {
// Retain for(;;) loop for performance reasons
// Refs: https://github.com/nodejs/node/pull/30958
for (let i = 0; i < value.length; i++)
storeHeader(self, state, key, value[i], validate);
return;
}
value = ArrayPrototypeJoin(value, '; ');
value = value.join('; ');
}
storeHeader(self, state, key, value, validate);
}
Expand All @@ -527,20 +512,20 @@ function storeHeader(self, state, key, value, validate) {
function matchHeader(self, state, field, value) {
if (field.length < 4 || field.length > 17)
return;
field = StringPrototypeToLowerCase(field);
field = field.toLowerCase();
switch (field) {
case 'connection':
state.connection = true;
self._removedConnection = false;
if (RegExpPrototypeTest(RE_CONN_CLOSE, value))
if (RE_CONN_CLOSE.test(value))
self._last = true;
else
self.shouldKeepAlive = true;
break;
case 'transfer-encoding':
state.te = true;
self._removedTE = false;
if (RegExpPrototypeTest(RE_TE_CHUNKED, value))
if (RE_TE_CHUNKED.test(value))
self.chunkedEncoding = true;
break;
case 'content-length':
Expand Down Expand Up @@ -583,9 +568,9 @@ OutgoingMessage.prototype.setHeader = function setHeader(name, value) {

let headers = this[kOutHeaders];
if (headers === null)
this[kOutHeaders] = headers = ObjectCreate(null);
this[kOutHeaders] = headers = Object.create(null);

headers[StringPrototypeToLowerCase(name)] = [name, value];
headers[name.toLowerCase()] = [name, value];
return this;
};

Expand All @@ -597,14 +582,14 @@ OutgoingMessage.prototype.getHeader = function getHeader(name) {
if (headers === null)
return;

const entry = headers[StringPrototypeToLowerCase(name)];
const entry = headers[name.toLowerCase()];
return entry && entry[1];
};


// Returns an array of the names of the current outgoing headers.
OutgoingMessage.prototype.getHeaderNames = function getHeaderNames() {
return this[kOutHeaders] !== null ? ObjectKeys(this[kOutHeaders]) : [];
return this[kOutHeaders] !== null ? Object.keys(this[kOutHeaders]) : [];
};


Expand All @@ -613,7 +598,7 @@ OutgoingMessage.prototype.getRawHeaderNames = function getRawHeaderNames() {
const headersMap = this[kOutHeaders];
if (headersMap === null) return [];

const values = ObjectValues(headersMap);
const values = Object.values(headersMap);
const headers = Array(values.length);
// Retain for(;;) loop for performance reasons
// Refs: https://github.com/nodejs/node/pull/30958
Expand All @@ -628,9 +613,9 @@ OutgoingMessage.prototype.getRawHeaderNames = function getRawHeaderNames() {
// Returns a shallow copy of the current outgoing headers.
OutgoingMessage.prototype.getHeaders = function getHeaders() {
const headers = this[kOutHeaders];
const ret = ObjectCreate(null);
const ret = Object.create(null);
if (headers) {
const keys = ObjectKeys(headers);
const keys = Object.keys(headers);
// Retain for(;;) loop for performance reasons
// Refs: https://github.com/nodejs/node/pull/30958
for (let i = 0; i < keys.length; ++i) {
Expand All @@ -646,7 +631,7 @@ OutgoingMessage.prototype.getHeaders = function getHeaders() {
OutgoingMessage.prototype.hasHeader = function hasHeader(name) {
validateString(name, 'name');
return this[kOutHeaders] !== null &&
!!this[kOutHeaders][StringPrototypeToLowerCase(name)];
!!this[kOutHeaders][name.toLowerCase()];
};


Expand All @@ -657,7 +642,7 @@ OutgoingMessage.prototype.removeHeader = function removeHeader(name) {
throw new ERR_HTTP_HEADERS_SENT('remove');
}

const key = StringPrototypeToLowerCase(name);
const key = name.toLowerCase();

switch (key) {
case 'connection':
Expand Down Expand Up @@ -783,7 +768,7 @@ function write_(msg, chunk, encoding, callback, fromEnd) {

let ret;
if (msg.chunkedEncoding && chunk.length !== 0) {
msg._send(NumberPrototypeToString(len, 16), 'latin1', null);
msg._send(len.toString(16), 'latin1', null);
msg._send(crlf_buf, null, null);
msg._send(chunk, encoding, null);
ret = msg._send(crlf_buf, null, callback);
Expand All @@ -803,8 +788,8 @@ function connectionCorkNT(conn) {

OutgoingMessage.prototype.addTrailers = function addTrailers(headers) {
this._trailer = '';
const keys = ObjectKeys(headers);
const isArray = ArrayIsArray(headers);
const keys = Object.keys(headers);
const isArray = Array.isArray(headers);
// Retain for(;;) loop for performance reasons
// Refs: https://github.com/nodejs/node/pull/30958
for (let i = 0, l = keys.length; i < l; i++) {
Expand Down Expand Up @@ -877,7 +862,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