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

errors: improve error creation performance #46648

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
14 changes: 0 additions & 14 deletions benchmark/error/error.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,23 @@ const common = require('../common.js');
const bench = common.createBenchmark(main, {
type: ['hide-stackframes-throw', 'direct-call-throw',
'hide-stackframes-noerr', 'direct-call-noerr'],
n: [10e4],
n: [1e4],
nested: [1, 0],
}, {
flags: ['--expose-internals'],
});

function main({ n, type }) {
function nestIt(fn, i = 25) {
const nested = (...args) => {
return fn(...args);
};
if (i === 0) {
return nested;
}
return nestIt(nested, i - 1);
}

function main({ n, type, nested }) {
const {
hideStackFrames,
codes: {
Expand All @@ -24,9 +35,14 @@ function main({ n, type }) {
}
};

let fn = testfn;
if (type.startsWith('hide-stackframe'))
fn = hideStackFrames(testfn);
let fn = type.startsWith('hide-stackframe') ?
hideStackFrames(testfn) :
testfn;

if (nested) {
fn = nestIt(fn);
}

let value = 42;
if (type.endsWith('-throw'))
value = 'err';
Expand Down
21 changes: 17 additions & 4 deletions benchmark/error/node-error.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,34 @@
'use strict';

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

const bench = common.createBenchmark(main, {
n: [1e7],
n: [1e5],
type: ['node', 'regular'],
}, {
flags: ['--expose-internals'],
});

function main({ n }) {
function main({ n, type }) {
const {
codes: {
ERR_INVALID_STATE,
},
} = require('internal/errors');

const Clazz = type === 'node' ? ERR_INVALID_STATE.TypeError : (() => {
class Foo extends TypeError {}
Foo.prototype.constructor = TypeError;
return Foo;
})();

bench.start();
for (let i = 0; i < n; ++i)
new ERR_INVALID_STATE.TypeError('test');
let length = 0;
for (let i = 0; i < n; i++) {
const error = new Clazz('test' + i);
length += error.name.length;
}
bench.end(n);
assert(length);
}
14 changes: 7 additions & 7 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2040,13 +2040,6 @@ which is not supported.
The input may not be used in the [`REPL`][]. The conditions under which this
error is used are described in the [`REPL`][] documentation.

<a id="ERR_INVALID_RETURN_PROPERTY"></a>

### `ERR_INVALID_RETURN_PROPERTY`

Thrown in case a function option does not provide a valid value for one of its
returned object properties on execution.

<a id="ERR_INVALID_RETURN_PROPERTY_VALUE"></a>

### `ERR_INVALID_RETURN_PROPERTY_VALUE`
Expand Down Expand Up @@ -3283,6 +3276,13 @@ removed: v15.0.0

An invalid or unknown file encoding was passed.

<a id="ERR_INVALID_RETURN_PROPERTY"></a>

### `ERR_INVALID_RETURN_PROPERTY`

Thrown in case a function option does not provide a valid value for one of its
returned object properties on execution.

<a id="ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST"></a>

### `ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`
Expand Down
13 changes: 6 additions & 7 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const {
ERR_INVALID_RETURN_VALUE,
ERR_MISSING_ARGS,
},
isErrorStackTraceLimitWritable,
setStackTraceLimit,
overrideStackTrace,
} = require('internal/errors');
const AssertionError = require('internal/assert/assertion_error');
Expand Down Expand Up @@ -282,16 +282,15 @@ function parseCode(code, offset) {
}

function getErrMessage(message, fn) {
const tmpLimit = Error.stackTraceLimit;
const errorStackTraceLimitIsWritable = isErrorStackTraceLimitWritable();
const { stackTraceLimit } = Error;
// Make sure the limit is set to 1. Otherwise it could fail (<= 0) or it
// does to much work.
if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = 1;
setStackTraceLimit(1);
// We only need the stack trace. To minimize the overhead use an object
// instead of an error.
const err = {};
ErrorCaptureStackTrace(err, fn);
if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = tmpLimit;
setStackTraceLimit(stackTraceLimit);

overrideStackTrace.set(err, (_, stack) => stack);
const call = err.stack[0];
Expand Down Expand Up @@ -323,7 +322,7 @@ function getErrMessage(message, fn) {
try {
// Set the stack trace limit to zero. This makes sure unexpected token
// errors are handled faster.
if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = 0;
setStackTraceLimit(0);

if (filename) {
if (decoder === undefined) {
Expand Down Expand Up @@ -368,7 +367,7 @@ function getErrMessage(message, fn) {
errorCache.set(identifier, undefined);
} finally {
// Reset limit.
if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = tmpLimit;
setStackTraceLimit(stackTraceLimit);
if (fd !== undefined)
closeSync(fd);
}
Expand Down
16 changes: 7 additions & 9 deletions lib/internal/assert/assertion_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const colors = require('internal/util/colors');
const {
validateObject,
} = require('internal/validators');
const { isErrorStackTraceLimitWritable } = require('internal/errors');
const { setStackTraceLimit } = require('internal/errors');


const kReadableOperator = {
Expand Down Expand Up @@ -337,8 +337,8 @@ class AssertionError extends Error {
expected
} = options;

const limit = Error.stackTraceLimit;
if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
const { stackTraceLimit } = Error;
setStackTraceLimit(0);

if (message != null) {
super(String(message));
Expand Down Expand Up @@ -421,16 +421,18 @@ class AssertionError extends Error {
}
}

if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
setStackTraceLimit(stackTraceLimit);

this.generatedMessage = !message;

ObjectDefineProperty(this, 'name', {
__proto__: null,
value: 'AssertionError [ERR_ASSERTION]',
value: 'AssertionError',
enumerable: false,
writable: true,
configurable: true
});

this.code = 'ERR_ASSERTION';
if (details) {
this.actual = undefined;
Expand All @@ -449,10 +451,6 @@ class AssertionError extends Error {
this.operator = operator;
}
ErrorCaptureStackTrace(this, stackStartFn || stackStartFunction);
// Create error message including the error code in the name.
this.stack; // eslint-disable-line no-unused-expressions
benjamingr marked this conversation as resolved.
Show resolved Hide resolved
// Reset the name.
this.name = 'AssertionError';
}

toString() {
Expand Down
5 changes: 4 additions & 1 deletion lib/internal/console/constructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,10 @@ const consoleMethods = {
trace: function trace(...args) {
const err = {
name: 'Trace',
message: this[kFormatForStderr](args)
message: this[kFormatForStderr](args),
toString() {
return `${this.name}: ${this.message}`;
}
BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
};
ErrorCaptureStackTrace(err, trace);
this.error(err.stack);
Expand Down
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(
benjamingr marked this conversation as resolved.
Show resolved Hide resolved
'info',
'must not contain more than 1024 bytes',
info.byteLength);
Expand Down
13 changes: 8 additions & 5 deletions lib/internal/debugger/inspect_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const {
ArrayPrototypePush,
ErrorCaptureStackTrace,
FunctionPrototypeBind,
JSONParse,
JSONStringify,
Expand All @@ -12,10 +11,15 @@ const {

const Buffer = require('buffer').Buffer;
const crypto = require('crypto');
const { ERR_DEBUGGER_ERROR } = require('internal/errors').codes;
const { EventEmitter } = require('events');
const http = require('http');
const URL = require('url');
const {
hideStackFrames,
codes: {
ERR_DEBUGGER_ERROR,
},
} = require('internal/errors');

const debuglog = require('internal/util/debuglog').debuglog('inspect');

Expand All @@ -40,12 +44,11 @@ const kMaskingKeyWidthInBytes = 4;
// https://tools.ietf.org/html/rfc6455#section-1.3
const WEBSOCKET_HANDSHAKE_GUID = '258EAFA5-E914-47DA-95CA-C5AB0DC85B11';

function unpackError({ code, message }) {
const unpackError = hideStackFrames(({ code, message }) => {
const err = new ERR_DEBUGGER_ERROR(`${message}`);
err.code = code;
ErrorCaptureStackTrace(err, unpackError);
return err;
}
});

function validateHandshake(requestKey, responseKey) {
const expectedResponseKeyBase = requestKey + WEBSOCKET_HANDSHAKE_GUID;
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/error_serdes.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ function deserializeError(error) {
return buf.toString('utf8');
}
}
require('assert').fail('This should not happen');
require('internal/assert').fail('This should not happen');
}

module.exports = { serializeError, deserializeError };
Loading