Skip to content

Commit

Permalink
errors: improve performance of instantiation
Browse files Browse the repository at this point in the history
PR-URL: #49654
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
  • Loading branch information
Uzlopak authored Sep 28, 2023
1 parent 3838b57 commit cc725a6
Show file tree
Hide file tree
Showing 18 changed files with 306 additions and 138 deletions.
File renamed without changes.
66 changes: 66 additions & 0 deletions benchmark/error/node-error-instantiation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
'use strict';

const common = require('../common');
const assert = require('assert');

const bench = common.createBenchmark(main, {
n: [1e6],
code: [
'built-in',
'ERR_HTTP2_STREAM_SELF_DEPENDENCY',
'ERR_INVALID_STATE',
'ERR_INVALID_URL',
],
stackTraceLimit: [0, 10],
}, {
flags: ['--expose-internals'],
});

function getErrorFactory(code) {
const {
ERR_HTTP2_STREAM_SELF_DEPENDENCY,
ERR_INVALID_STATE,
ERR_INVALID_URL,
} = require('internal/errors').codes;

switch (code) {
case 'built-in':
return (n) => new Error();
case 'ERR_HTTP2_STREAM_SELF_DEPENDENCY':
return (n) => new ERR_HTTP2_STREAM_SELF_DEPENDENCY();
case 'ERR_INVALID_STATE':
return (n) => new ERR_INVALID_STATE(n + '');
case 'ERR_INVALID_URL':
return (n) => new ERR_INVALID_URL({ input: n + '' });
default:
throw new Error(`${code} not supported`);
}
}

function main({ n, code, stackTraceLimit }) {
const getError = getErrorFactory(code);

Error.stackTraceLimit = stackTraceLimit;

// Warm up.
const length = 1024;
const array = [];
for (let i = 0; i < length; ++i) {
array.push(getError(i));
}

bench.start();

for (let i = 0; i < n; ++i) {
const index = i % length;
array[index] = getError(index);
}

bench.end(n);

// Verify the entries to prevent dead code elimination from making
// the benchmark invalid.
for (let i = 0; i < length; ++i) {
assert.strictEqual(typeof array[i], 'object');
}
}
62 changes: 62 additions & 0 deletions benchmark/error/node-error-stack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
'use strict';

const common = require('../common');
const assert = require('assert');

const bench = common.createBenchmark(main, {
n: [1e6],
code: [
'built-in',
'ERR_HTTP2_STREAM_SELF_DEPENDENCY',
'ERR_INVALID_STATE',
],
stackTraceLimit: [0, 10],
}, {
flags: ['--expose-internals'],
});

function getErrorStackFactory(code) {
const {
ERR_INVALID_STATE,
ERR_HTTP2_STREAM_SELF_DEPENDENCY,
} = require('internal/errors').codes;

switch (code) {
case 'built-in':
return (n) => new Error().stack;
case 'ERR_HTTP2_STREAM_SELF_DEPENDENCY':
return (n) => new ERR_HTTP2_STREAM_SELF_DEPENDENCY().stack;
case 'ERR_INVALID_STATE':
return (n) => new ERR_INVALID_STATE(n + '').stack;
default:
throw new Error(`${code} not supported`);
}
}

function main({ n, code, stackTraceLimit }) {
const getStack = getErrorStackFactory(code);

Error.stackTraceLimit = stackTraceLimit;

// Warm up.
const length = 1024;
const array = [];
for (let i = 0; i < length; ++i) {
array.push(getStack(i));
}

bench.start();

for (let i = 0; i < n; ++i) {
const index = i % length;
array[index] = getStack(index);
}

bench.end(n);

// Verify the entries to prevent dead code elimination from making
// the benchmark invalid.
for (let i = 0; i < length; ++i) {
assert.strictEqual(typeof array[i], 'string');
}
}
21 changes: 0 additions & 21 deletions benchmark/error/node-error.js

This file was deleted.

2 changes: 1 addition & 1 deletion lib/internal/crypto/hkdf.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const validateParameters = hideStackFrames((hash, key, salt, info, length) => {
validateInteger(length, 'length', 0, kMaxLength);

if (info.byteLength > 1024) {
throw ERR_OUT_OF_RANGE(
throw new ERR_OUT_OF_RANGE(
'info',
'must not contain more than 1024 bytes',
info.byteLength);
Expand Down
150 changes: 107 additions & 43 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,10 @@ const aggregateErrors = hideStackFrames((errors, message, code) => {
return err;
});

const assert = require('internal/assert');

// Lazily loaded
let util;
let assert;

let internalUtil = null;
function lazyInternalUtil() {
Expand Down Expand Up @@ -371,42 +372,103 @@ function makeSystemErrorWithCode(key) {
}

function makeNodeErrorWithCode(Base, key) {
return function NodeError(...args) {
const limit = Error.stackTraceLimit;
if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
const error = new Base();
// Reset the limit and setting the name property.
if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
const message = getMessage(key, args, error);
ObjectDefineProperties(error, {
[kIsNodeError]: {
__proto__: null,
value: true,
enumerable: false,
writable: false,
configurable: true,
},
message: {
__proto__: null,
value: message,
enumerable: false,
writable: true,
configurable: true,
},
toString: {
__proto__: null,
value() {
const msg = messages.get(key);
const expectedLength = typeof msg !== 'string' ? -1 : getExpectedArgumentLength(msg);

switch (expectedLength) {
case 0: {
class NodeError extends Base {
code = key;

constructor(...args) {
assert(
args.length === 0,
`Code: ${key}; The provided arguments length (${args.length}) does not ` +
`match the required ones (${expectedLength}).`,
);
super(msg);
}

// This is a workaround for wpt tests that expect that the error
// constructor has a `name` property of the base class.
get ['constructor']() {
return Base;
}

get [kIsNodeError]() {
return true;
}

toString() {
return `${this.name} [${key}]: ${this.message}`;
},
enumerable: false,
writable: true,
configurable: true,
},
});
captureLargerStackTrace(error);
error.code = key;
return error;
};
}
}
return NodeError;
}
case -1: {
class NodeError extends Base {
code = key;

constructor(...args) {
super();
ObjectDefineProperty(this, 'message', {
__proto__: null,
value: getMessage(key, args, this),
enumerable: false,
writable: true,
configurable: true,
});
}

// This is a workaround for wpt tests that expect that the error
// constructor has a `name` property of the base class.
get ['constructor']() {
return Base;
}

get [kIsNodeError]() {
return true;
}

toString() {
return `${this.name} [${key}]: ${this.message}`;
}
}
return NodeError;
}
default: {

class NodeError extends Base {
code = key;

constructor(...args) {
assert(
args.length === expectedLength,
`Code: ${key}; The provided arguments length (${args.length}) does not ` +
`match the required ones (${expectedLength}).`,
);

ArrayPrototypeUnshift(args, msg);
super(ReflectApply(lazyInternalUtilInspect().format, null, args));
}

// This is a workaround for wpt tests that expect that the error
// constructor has a `name` property of the base class.
get ['constructor']() {
return Base;
}

get [kIsNodeError]() {
return true;
}

toString() {
return `${this.name} [${key}]: ${this.message}`;
}
}
return NodeError;
}
}
}

/**
Expand Down Expand Up @@ -443,11 +505,16 @@ function E(sym, val, def, ...otherClasses) {
codes[sym] = def;
}

function getExpectedArgumentLength(msg) {
let expectedLength = 0;
const regex = /%[dfijoOs]/g;
while (RegExpPrototypeExec(regex, msg) !== null) expectedLength++;
return expectedLength;
}

function getMessage(key, args, self) {
const msg = messages.get(key);

assert ??= require('internal/assert');

if (typeof msg === 'function') {
assert(
msg.length <= args.length, // Default options do not count.
Expand All @@ -457,9 +524,7 @@ function getMessage(key, args, self) {
return ReflectApply(msg, self, args);
}

const regex = /%[dfijoOs]/g;
let expectedLength = 0;
while (RegExpPrototypeExec(regex, msg) !== null) expectedLength++;
const expectedLength = getExpectedArgumentLength(msg);
assert(
expectedLength === args.length,
`Code: ${key}; The provided arguments length (${args.length}) does not ` +
Expand Down Expand Up @@ -1476,8 +1541,7 @@ E('ERR_NETWORK_IMPORT_DISALLOWED',
"import of '%s' by %s is not supported: %s", Error);
E('ERR_NOT_BUILDING_SNAPSHOT',
'Operation cannot be invoked when not building startup snapshot', Error);
E('ERR_NOT_SUPPORTED_IN_SNAPSHOT',
'%s is not supported in startup snapshot', Error);
E('ERR_NOT_SUPPORTED_IN_SNAPSHOT', '%s is not supported in startup snapshot', Error);
E('ERR_NO_CRYPTO',
'Node.js is not compiled with OpenSSL crypto support', Error);
E('ERR_NO_ICU',
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/fs/streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ function importFd(stream, options) {
return options.fd.fd;
}

throw ERR_INVALID_ARG_TYPE('options.fd',
['number', 'FileHandle'], options.fd);
throw new ERR_INVALID_ARG_TYPE('options.fd',
['number', 'FileHandle'], options.fd);
}

function ReadStream(path, options) {
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ class Hooks {
!isAnyArrayBuffer(source) &&
!isArrayBufferView(source)
) {
throw ERR_INVALID_RETURN_PROPERTY_VALUE(
throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
'a string, an ArrayBuffer, or a TypedArray',
hookErrIdentifier,
'source',
Expand Down Expand Up @@ -599,7 +599,7 @@ class HooksProxy {
if (status === 'error') {
if (body == null || typeof body !== 'object') { throw body; }
if (body.serializationFailed || body.serialized == null) {
throw ERR_WORKER_UNSERIALIZABLE_ERROR();
throw new ERR_WORKER_UNSERIALIZABLE_ERROR();
}

// eslint-disable-next-line no-restricted-syntax
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/navigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class Navigator {
if (arguments[0] === kInitialize) {
return;
}
throw ERR_ILLEGAL_CONSTRUCTOR();
throw new ERR_ILLEGAL_CONSTRUCTOR();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ class URL {
set href(value) {
value = `${value}`;
const href = bindingUrl.update(this.#context.href, updateActions.kHref, value);
if (!href) { throw ERR_INVALID_URL(value); }
if (!href) { throw new ERR_INVALID_URL(value); }
this.#updateContext(href);
}

Expand Down
Loading

0 comments on commit cc725a6

Please sign in to comment.