errors: improve performance of instantiation

PR-URL: nodejs#49654 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Raz Luvaton <[email protected]>
debadree25 · Apr 15, 2024 · 9e04f2c · 9e04f2c
1 parent 6575fac
commit 9e04f2c
Show file tree

Hide file tree

Showing 18 changed files with 306 additions and 138 deletions.
diff --git a/benchmark/misc/hidestackframes.js → benchmark/error/hidestackframes.js b/benchmark/misc/hidestackframes.js → benchmark/error/hidestackframes.js
diff --git a/benchmark/error/node-error-instantiation.js b/benchmark/error/node-error-instantiation.js
@@ -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');
+  }
+}
diff --git a/benchmark/error/node-error-stack.js b/benchmark/error/node-error-stack.js
@@ -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');
+  }
+}
diff --git a/benchmark/error/node-error.js b/benchmark/error/node-error.js
diff --git a/lib/internal/crypto/hkdf.js b/lib/internal/crypto/hkdf.js
@@ -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);

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -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() {
@@ -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;
+    }
+  }
 }
 
 /**
@@ -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.
@@ -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 ` +
@@ -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',

diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js
@@ -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) {

diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
@@ -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',
@@ -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

diff --git a/lib/internal/navigator.js b/lib/internal/navigator.js
@@ -27,7 +27,7 @@ class Navigator {
     if (arguments[0] === kInitialize) {
       return;
     }
-    throw ERR_ILLEGAL_CONSTRUCTOR();
+    throw new ERR_ILLEGAL_CONSTRUCTOR();
   }
 
   /**

diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -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);
   }