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.

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

lib: revert primordials in a hot path #38246

Closed
wants to merge 8 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixup
  • Loading branch information
mcollina committed Apr 14, 2021
commit 590a5453562e1a95c6012714b2c6c34261324187
14 changes: 7 additions & 7 deletions lib/events.js
Original file line number Diff line number Diff line change
@@ -146,7 +146,7 @@ ObjectDefineProperties(EventEmitter, {

EventEmitter.setMaxListeners =
function(n = defaultMaxListeners, ...eventTargets) {
if (typeof n !== 'number' || n < 0 || NumberIsNaN(n))
if (typeof n !== 'number' || n < 0 || Number.isNaN(n))
throw new ERR_OUT_OF_RANGE('n', 'a non-negative number', n);
if (eventTargets.length === 0) {
defaultMaxListeners = n;
@@ -241,7 +241,7 @@ function emitUnhandledRejectionOrErr(ee, err, type, args) {
// Obviously not all Emitters should be limited to 10. This function allows
// that to be increased. Set to zero for unlimited.
EventEmitter.prototype.setMaxListeners = function setMaxListeners(n) {
if (typeof n !== 'number' || n < 0 || NumberIsNaN(n)) {
if (typeof n !== 'number' || n < 0 || Number.isNaN(n)) {
throw new ERR_OUT_OF_RANGE('n', 'a non-negative number', n);
}
this._maxListeners = n;
@@ -392,7 +392,7 @@ function _addListener(target, type, listener, prepend) {

events = target._events;
if (events === undefined) {
events = target._events = ObjectCreate(null);
events = target._events = Object.create(null);
target._eventsCount = 0;
} else {
// To avoid recursion in the case that type === "newListener"! Before
@@ -504,7 +504,7 @@ EventEmitter.prototype.removeListener =

if (list === listener || list.listener === listener) {
if (--this._eventsCount === 0)
this._events = ObjectCreate(null);
this._events = Object.create(null);
else {
delete events[type];
if (events.removeListener)
@@ -552,11 +552,11 @@ EventEmitter.prototype.removeAllListeners =
// Not listening for removeListener, no need to emit
if (events.removeListener === undefined) {
if (arguments.length === 0) {
this._events = ObjectCreate(null);
this._events = Object.create(null);
this._eventsCount = 0;
} else if (events[type] !== undefined) {
if (--this._eventsCount === 0)
this._events = ObjectCreate(null);
this._events = Object.create(null);
else
delete events[type];
}
@@ -570,7 +570,7 @@ EventEmitter.prototype.removeAllListeners =
this.removeAllListeners(key);
}
this.removeAllListeners('removeListener');
this._events = ObjectCreate(null);
this._events = Object.create(null);
this._eventsCount = 0;
return this;
}
29 changes: 11 additions & 18 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
'use strict';

const {
ArrayPrototypePop,
ArrayPrototypeSlice,
ArrayPrototypeUnshift,
ErrorCaptureStackTrace,
FunctionPrototypeBind,
ObjectPrototypeHasOwnProperty,
ObjectDefineProperty,
Promise,
ReflectApply,
Copy link
Member Author

Choose a reason for hiding this comment

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

@aduh95 the entirety of this file is a critical hot path.

Symbol,
} = primordials;

@@ -129,16 +122,16 @@ function callbackTrampoline(asyncId, resource, cb, ...args) {

let result;
if (asyncId === 0 && typeof domain_cb === 'function') {
ArrayPrototypeUnshift(args, cb);
result = ReflectApply(domain_cb, this, args);
args.unshift(cb);
result = Reflect.apply(domain_cb, this, args);
} else {
result = ReflectApply(cb, this, args);
result = Reflect.apply(cb, this, args);
}

if (asyncId !== 0 && hasHooks(kAfter))
emitAfterNative(asyncId);

ArrayPrototypePop(execution_async_resources);
execution_async_resources.pop();
return result;
}

@@ -170,7 +163,7 @@ function fatalError(e) {
process._rawDebug(e.stack);
} else {
const o = inspectExceptionValue(e);
ErrorCaptureStackTrace(o, fatalError);
Error.captureStackTrace(o, fatalError);
process._rawDebug(o.stack);
}

@@ -256,7 +249,7 @@ function emitHook(symbol, asyncId) {
}

function emitHookFactory(symbol, name) {
const fn = FunctionPrototypeBind(emitHook, undefined, symbol);
const fn = emitHook.bind(undefined, symbol);

// Set the name property of the function as it looks good in the stack trace.
ObjectDefineProperty(fn, 'name', {
@@ -281,7 +274,7 @@ function getHookArrays() {


function storeActiveHooks() {
active_hooks.tmp_array = ArrayPrototypeSlice(active_hooks.array);
active_hooks.tmp_array = active_hooks.array.slice();
// Don't want to make the assumption that kInit to kDestroy are indexes 0 to
// 4. So do this the long way.
active_hooks.tmp_fields = [];
@@ -402,7 +395,7 @@ function newAsyncId() {
}

function getOrSetAsyncId(object) {
if (ObjectPrototypeHasOwnProperty(object, async_id_symbol)) {
if (object.hasOwnProperty(async_id_symbol)) {
return object[async_id_symbol];
}

@@ -429,14 +422,14 @@ function clearDefaultTriggerAsyncId() {

function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) {
if (triggerAsyncId === undefined)
return ReflectApply(block, null, args);
return Reflect.apply(block, null, args);
// CHECK(NumberIsSafeInteger(triggerAsyncId))
// CHECK(triggerAsyncId > 0)
const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId];
async_id_fields[kDefaultTriggerAsyncId] = triggerAsyncId;

try {
return ReflectApply(block, null, args);
return Reflect.apply(block, null, args);
} finally {
async_id_fields[kDefaultTriggerAsyncId] = oldDefaultTriggerAsyncId;
}
@@ -533,7 +526,7 @@ function popAsyncContext(asyncId) {
const offset = stackLength - 1;
async_id_fields[kExecutionAsyncId] = async_wrap.async_ids_stack[2 * offset];
async_id_fields[kTriggerAsyncId] = async_wrap.async_ids_stack[2 * offset + 1];
ArrayPrototypePop(execution_async_resources);
execution_async_resources.pop();
async_hook_fields[kStackLength] = offset;
return offset > 0;
}
10 changes: 5 additions & 5 deletions lib/internal/buffer.js
Original file line number Diff line number Diff line change
@@ -66,7 +66,7 @@ function checkInt(value, min, max, buf, offset, byteLength) {
}

function boundsError(value, length, type) {
if (MathFloor(value) !== value) {
if (Math.floor(value) !== value) {
validateNumber(value, type);
throw new ERR_OUT_OF_RANGE(type || 'offset', 'an integer', value);
}
@@ -649,7 +649,7 @@ function writeU_Int48LE(buf, value, offset, min, max) {
value = +value;
checkInt(value, min, max, buf, offset, 5);

const newVal = MathFloor(value * 2 ** -32);
const newVal = Math.floor(value * 2 ** -32);
buf[offset++] = value;
value = value >>> 8;
buf[offset++] = value;
@@ -674,7 +674,7 @@ function writeU_Int40LE(buf, value, offset, min, max) {
buf[offset++] = value;
value = value >>> 8;
buf[offset++] = value;
buf[offset++] = MathFloor(newVal * 2 ** -32);
buf[offset++] = Math.floor(newVal * 2 ** -32);
return offset;
}

@@ -760,7 +760,7 @@ function writeU_Int48BE(buf, value, offset, min, max) {
value = +value;
checkInt(value, min, max, buf, offset, 5);

const newVal = MathFloor(value * 2 ** -32);
const newVal = Math.floor(value * 2 ** -32);
buf[offset++] = (newVal >>> 8);
buf[offset++] = newVal;
buf[offset + 3] = value;
@@ -777,7 +777,7 @@ function writeU_Int40BE(buf, value, offset, min, max) {
value = +value;
checkInt(value, min, max, buf, offset, 4);

buf[offset++] = MathFloor(value * 2 ** -32);
buf[offset++] = Math.floor(value * 2 ** -32);
buf[offset + 3] = value;
value = value >>> 8;
buf[offset + 2] = value;
18 changes: 6 additions & 12 deletions lib/internal/timers.js
Original file line number Diff line number Diff line change
@@ -73,13 +73,7 @@
// have shown to be trivial in comparison to other timers architectures.

const {
MathMax,
MathTrunc,
NumberIsFinite,
NumberMIN_SAFE_INTEGER,
ObjectCreate,
Symbol,
ReflectApply,
} = primordials;

const {
@@ -132,7 +126,7 @@ const kHasOutstanding = 2;
// Timeout values > TIMEOUT_MAX are set to 1.
const TIMEOUT_MAX = 2 ** 31 - 1;

let timerListId = NumberMIN_SAFE_INTEGER;
let timerListId = Number.MIN_SAFE_INTEGER;

const kRefed = Symbol('refed');

@@ -152,7 +146,7 @@ const timerListQueue = new PriorityQueue(compareTimersLists, setPosition);
//
// - key = time in milliseconds
// - value = linked list
const timerListMap = ObjectCreate(null);
const timerListMap = Object.create(null);

function initAsyncResource(resource, type) {
const asyncId = resource[async_id_symbol] = newAsyncId();
@@ -349,7 +343,7 @@ function insertGuarded(item, refed, start) {

function insert(item, msecs, start = getLibuvNow()) {
// Truncate so that accuracy of sub-millisecond timers is not assumed.
msecs = MathTrunc(msecs);
msecs = Math.trunc(msecs);
item._idleStart = start;

// Use an existing list if there is one, otherwise we need to make a new one.
@@ -382,7 +376,7 @@ function setUnrefTimeout(callback, after) {
// Type checking used by timers.enroll() and Socket#setTimeout()
function getTimerDuration(msecs, name) {
validateNumber(msecs, name);
if (msecs < 0 || !NumberIsFinite(msecs)) {
if (msecs < 0 || !Number.isFinite(msecs)) {
throw new ERR_OUT_OF_RANGE(name, 'a non-negative finite number', msecs);
}

@@ -515,7 +509,7 @@ function getTimerCallbacks(runNextTicks) {
// Check if this loop iteration is too early for the next timer.
// This happens if there are more timers scheduled for later in the list.
if (diff < msecs) {
list.expiry = MathMax(timer._idleStart + msecs, now + 1);
list.expiry = Math.max(timer._idleStart + msecs, now + 1);
list.id = timerListId++;
timerListQueue.percolateDown(1);
debug('%d list wait because diff is %d', msecs, diff);
@@ -556,7 +550,7 @@ function getTimerCallbacks(runNextTicks) {
if (args === undefined)
timer._onTimeout();
else
ReflectApply(timer._onTimeout, timer, args);
Reflect.apply(timer._onTimeout, timer, args);
} finally {
if (timer._repeat && timer._idleTimeout !== -1) {
timer._idleTimeout = timer._repeat;
2 changes: 1 addition & 1 deletion lib/net.js
Original file line number Diff line number Diff line change
@@ -907,7 +907,7 @@ Socket.prototype.connect = function(...args) {
// already been normalized (so we don't normalize more than once). This has
// been solved before in https://github.com/nodejs/node/pull/12342, but was
// reverted as it had unintended side effects.
if (ArrayIsArray(args[0]) && args[0][normalizedArgsSymbol]) {
if (Array.isArray(args[0]) && args[0][normalizedArgsSymbol]) {
normalized = args[0];
} else {
normalized = normalizeArgs(args);