From f0ac334138648ff1f639c7394bd3eddf7b5ed586 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 18 Nov 2018 03:50:13 +0100 Subject: [PATCH 1/2] 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. --- 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 3bedce9d515b38..0dff3e1a6e4583 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -445,7 +445,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 + // loose 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; } @@ -453,12 +459,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 0ed23a943805a8..04c68c2af21874 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 637b9ffb54b406..43a9fefd9d5b1e 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; From e006df41fab4f4e492d5afe75ff9db3034ebab47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Fri, 30 Nov 2018 15:48:49 +0100 Subject: [PATCH 2/2] fixup: fix typo Co-Authored-By: BridgeAR --- lib/_http_outgoing.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 0dff3e1a6e4583..6e1721f9292875 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -446,7 +446,7 @@ 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 - // loose the stack frames due to the `captureStackTrace()` function that is + // lose the stack frames due to the `captureStackTrace()` function that is // called later. const tmpLimit = Error.stackTraceLimit; Error.stackTraceLimit = 0;