From a1a5c0419e62f9ae3bbef9fb04fac2d867c9e509 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 18 Nov 2018 03:50:13 +0100 Subject: [PATCH] lib: improve error creation performance In case of an error where we only care about a cleaned up stack trace it is cheaper to reset the stack trace limit for the error that is created. That way the stack frames do not have to be computed twice. PR-URL: https://github.com/nodejs/node/pull/24747 Reviewed-By: Anna Henningsen Reviewed-By: Jeremiah Senkpiel --- lib/_http_outgoing.js | 12 ++++++++++++ lib/internal/errors.js | 24 ++++++++++++++++++++++++ lib/internal/fs/utils.js | 16 ++++++++++------ lib/internal/process/warning.js | 5 +++++ 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index eb68d091c68407..80125f8c24f04b 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -446,7 +446,13 @@ function matchHeader(self, state, field, value) { function validateHeaderName(name) { if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) { + // Reducing the limit improves the performance significantly. We do not + // lose the stack frames due to the `captureStackTrace()` function that is + // called later. + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; const err = new ERR_INVALID_HTTP_TOKEN('Header name', name); + Error.stackTraceLimit = tmpLimit; Error.captureStackTrace(err, validateHeaderName); throw err; } @@ -454,12 +460,18 @@ function validateHeaderName(name) { function validateHeaderValue(name, value) { let err; + // Reducing the limit improves the performance significantly. We do not loose + // the stack frames due to the `captureStackTrace()` function that is called + // later. + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; if (value === undefined) { err = new ERR_HTTP_INVALID_HEADER_VALUE(value, name); } else if (checkInvalidHeaderChar(value)) { debug('Header "%s" contains invalid characters', name); err = new ERR_INVALID_CHAR('header content', name); } + Error.stackTraceLimit = tmpLimit; if (err !== undefined) { Error.captureStackTrace(err, validateHeaderValue); throw err; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index f497a0d6e4a0b8..b8f8b4cfa2d725 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -261,10 +261,16 @@ function uvException(ctx) { message += ` -> '${dest}'`; } + // Reducing the limit improves the performance significantly. We do not loose + // the stack frames due to the `captureStackTrace()` function that is called + // later. + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; // Pass the message to the constructor instead of setting it on the object // to make sure it is the same as the one created in C++ // eslint-disable-next-line no-restricted-syntax const err = new Error(message); + Error.stackTraceLimit = tmpLimit; for (const prop of Object.keys(ctx)) { if (prop === 'message' || prop === 'path' || prop === 'dest') { @@ -307,8 +313,14 @@ function uvExceptionWithHostPort(err, syscall, address, port) { details = ` ${address}`; } + // Reducing the limit improves the performance significantly. We do not loose + // the stack frames due to the `captureStackTrace()` function that is called + // later. + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; // eslint-disable-next-line no-restricted-syntax const ex = new Error(`${message}${details}`); + Error.stackTraceLimit = tmpLimit; ex.code = code; ex.errno = code; ex.syscall = syscall; @@ -377,9 +389,15 @@ function exceptionWithHostPort(err, syscall, address, port, additional) { details += ` - Local (${additional})`; } + // Reducing the limit improves the performance significantly. We do not loose + // the stack frames due to the `captureStackTrace()` function that is called + // later. + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; // eslint-disable-next-line no-restricted-syntax const ex = new Error(`${syscall} ${code}${details}`); // TODO(joyeecheung): errno is supposed to err, like in uvException + Error.stackTraceLimit = tmpLimit; ex.code = ex.errno = code; ex.syscall = syscall; ex.address = address; @@ -410,9 +428,15 @@ function dnsException(code, syscall, hostname) { } } const message = `${syscall} ${code}${hostname ? ` ${hostname}` : ''}`; + // Reducing the limit improves the performance significantly. We do not loose + // the stack frames due to the `captureStackTrace()` function that is called + // later. + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; // eslint-disable-next-line no-restricted-syntax const ex = new Error(message); // TODO(joyeecheung): errno is supposed to be a number / err, like in + Error.stackTraceLimit = tmpLimit; // uvException. ex.errno = code; ex.code = code; diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 0df13354acc628..4db4fb536c0b6a 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -190,16 +190,19 @@ function nullCheck(path, propName, throwError = true) { const pathIsUint8Array = isUint8Array(path); // We can only perform meaningful checks on strings and Uint8Arrays. - if (!pathIsString && !pathIsUint8Array) { + if (!pathIsString && !pathIsUint8Array || + pathIsString && path.indexOf('\u0000') === -1 || + pathIsUint8Array && path.indexOf(0) === -1) { return; } - if (pathIsString && path.indexOf('\u0000') === -1) { - return; - } else if (pathIsUint8Array && path.indexOf(0) === -1) { - return; + // Reducing the limit improves the performance significantly. We do not loose + // the stack frames due to the `captureStackTrace()` function that is called + // later. + const tmpLimit = Error.stackTraceLimit; + if (throwError) { + Error.stackTraceLimit = 0; } - const err = new ERR_INVALID_ARG_VALUE( propName, path, @@ -207,6 +210,7 @@ function nullCheck(path, propName, throwError = true) { ); if (throwError) { + Error.stackTraceLimit = tmpLimit; Error.captureStackTrace(err, nullCheck); throw err; } diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js index 5dac78082bf205..2238bc2577bb50 100644 --- a/lib/internal/process/warning.js +++ b/lib/internal/process/warning.js @@ -127,8 +127,13 @@ function setupProcessWarnings() { throw new ERR_INVALID_ARG_TYPE('code', 'string', code); } if (typeof warning === 'string') { + // Improve error creation performance by skipping the error frames. + // They are added in the `captureStackTrace()` function below. + const tmpStackLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; // eslint-disable-next-line no-restricted-syntax warning = new Error(warning); + Error.stackTraceLimit = tmpStackLimit; warning.name = String(type || 'Warning'); if (code !== undefined) warning.code = code; if (detail !== undefined) warning.detail = detail;