lib: reduce overhead of validateObject

H4ad · targos · commit 24cbc550c2aa · 2023-11-11T19:39:29.000+01:00
PR-URL: #49928 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
diff --git a/benchmark/validators/validate-object.js b/benchmark/validators/validate-object.js
@@ -0,0 +1,75 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+const bench = common.createBenchmark(main, {
+  n: [1e5],
+  objectToTest: [
+    'object',
+    'null',
+    'array',
+    'function',
+  ],
+}, {
+  flags: ['--expose-internals'],
+});
+
+function getObjectToTest(objectToTest) {
+  switch (objectToTest) {
+    case 'object':
+      return { foo: 'bar' };
+    case 'null':
+      return null;
+    case 'array':
+      return ['foo', 'bar'];
+    case 'function':
+      return () => 'foo';
+    default:
+      throw new Error(`Value ${objectToTest} is not a valid objectToTest.`);
+  }
+}
+
+function getOptions(objectToTest) {
+  const {
+    kValidateObjectAllowNullable,
+    kValidateObjectAllowArray,
+    kValidateObjectAllowFunction,
+  } = require('internal/validators');
+
+  switch (objectToTest) {
+    case 'object':
+      return 0;
+    case 'null':
+      return kValidateObjectAllowNullable;
+    case 'array':
+      return kValidateObjectAllowArray;
+    case 'function':
+      return kValidateObjectAllowFunction;
+    default:
+      throw new Error(`Value ${objectToTest} is not a valid objectToTest.`);
+  }
+}
+
+let _validateResult;
+
+function main({ n, objectToTest }) {
+  const {
+    validateObject,
+  } = require('internal/validators');
+
+  const value = getObjectToTest(objectToTest);
+  const options = getOptions(objectToTest);
+
+  bench.start();
+  for (let i = 0; i < n; ++i) {
+    try {
+      _validateResult = validateObject(value, 'Object', options);
+    } catch {
+      _validateResult = undefined;
+    }
+  }
+  bench.end(n);
+
+  assert.ok(!_validateResult);
+}
diff --git a/lib/fs.js b/lib/fs.js
@@ -140,6 +140,7 @@ const {
   validateInteger,
   validateObject,
   validateString,
+  kValidateObjectAllowNullable,
 } = require('internal/validators');
 
 let truncateWarn = true;
@@ -624,7 +625,7 @@ function read(fd, buffer, offsetOrOptions, length, position, callback) {
   if (arguments.length <= 4) {
     if (arguments.length === 4) {
       // This is fs.read(fd, buffer, options, callback)
-      validateObject(offsetOrOptions, 'options', { nullable: true });
+      validateObject(offsetOrOptions, 'options', kValidateObjectAllowNullable);
       callback = length;
       params = offsetOrOptions;
     } else if (arguments.length === 3) {
@@ -642,7 +643,7 @@ function read(fd, buffer, offsetOrOptions, length, position, callback) {
     }
 
     if (params !== undefined) {
-      validateObject(params, 'options', { nullable: true });
+      validateObject(params, 'options', kValidateObjectAllowNullable);
     }
     ({
       offset = 0,
@@ -714,7 +715,7 @@ function readSync(fd, buffer, offsetOrOptions, length, position) {
   let offset = offsetOrOptions;
   if (arguments.length <= 3 || typeof offsetOrOptions === 'object') {
     if (offsetOrOptions !== undefined) {
-      validateObject(offsetOrOptions, 'options', { nullable: true });
+      validateObject(offsetOrOptions, 'options', kValidateObjectAllowNullable);
     }
 
     ({
diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js
@@ -46,6 +46,8 @@ const {
   validateAbortSignalArray,
   validateObject,
   validateUint32,
+  kValidateObjectAllowArray,
+  kValidateObjectAllowFunction,
 } = require('internal/validators');
 
 const {
@@ -432,7 +434,7 @@ async function aborted(signal, resource) {
     throw new ERR_INVALID_ARG_TYPE('signal', 'AbortSignal', signal);
   }
   validateAbortSignal(signal, 'signal');
-  validateObject(resource, 'resource', { nullable: false, allowFunction: true, allowArray: true });
+  validateObject(resource, 'resource', kValidateObjectAllowArray | kValidateObjectAllowFunction);
   if (signal.aborted)
     return PromiseResolve();
   const abortPromise = createDeferredPromise();
diff --git a/lib/internal/encoding.js b/lib/internal/encoding.js
@@ -47,6 +47,9 @@ const {
 const {
   validateString,
   validateObject,
+  kValidateObjectAllowNullable,
+  kValidateObjectAllowArray,
+  kValidateObjectAllowFunction,
 } = require('internal/validators');
 const binding = internalBinding('encoding_binding');
 const {
@@ -390,6 +393,10 @@ const TextDecoder =
     makeTextDecoderICU() :
     makeTextDecoderJS();
 
+const kValidateObjectAllowObjectsAndNull = kValidateObjectAllowNullable |
+  kValidateObjectAllowArray |
+  kValidateObjectAllowFunction;
+
 function makeTextDecoderICU() {
   const {
     decode: _decode,
@@ -399,11 +406,7 @@ function makeTextDecoderICU() {
   class TextDecoder {
     constructor(encoding = 'utf-8', options = kEmptyObject) {
       encoding = `${encoding}`;
-      validateObject(options, 'options', {
-        nullable: true,
-        allowArray: true,
-        allowFunction: true,
-      });
+      validateObject(options, 'options', kValidateObjectAllowObjectsAndNull);
 
       const enc = getEncodingFromLabel(encoding);
       if (enc === undefined)
@@ -448,11 +451,7 @@ function makeTextDecoderICU() {
 
       this.#prepareConverter();
 
-      validateObject(options, 'options', {
-        nullable: true,
-        allowArray: true,
-        allowFunction: true,
-      });
+      validateObject(options, 'options', kValidateObjectAllowObjectsAndNull);
 
       let flags = 0;
       if (options !== null)
@@ -482,11 +481,7 @@ function makeTextDecoderJS() {
   class TextDecoder {
     constructor(encoding = 'utf-8', options = kEmptyObject) {
       encoding = `${encoding}`;
-      validateObject(options, 'options', {
-        nullable: true,
-        allowArray: true,
-        allowFunction: true,
-      });
+      validateObject(options, 'options', kValidateObjectAllowObjectsAndNull);
 
       const enc = getEncodingFromLabel(encoding);
       if (enc === undefined || !hasConverter(enc))
@@ -528,11 +523,7 @@ function makeTextDecoderJS() {
                                        ['ArrayBuffer', 'ArrayBufferView'],
                                        input);
       }
-      validateObject(options, 'options', {
-        nullable: true,
-        allowArray: true,
-        allowFunction: true,
-      });
+      validateObject(options, 'options', kValidateObjectAllowObjectsAndNull);
 
       if (this[kFlags] & CONVERTER_FLAGS_FLUSH) {
         this[kBOMSeen] = false;
diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
@@ -34,7 +34,14 @@ const {
     ERR_INVALID_THIS,
   },
 } = require('internal/errors');
-const { validateAbortSignal, validateObject, validateString, validateInternalField } = require('internal/validators');
+const {
+  validateAbortSignal,
+  validateObject,
+  validateString,
+  validateInternalField,
+  kValidateObjectAllowArray,
+  kValidateObjectAllowFunction,
+} = require('internal/validators');
 
 const {
   customInspectSymbol,
@@ -1041,9 +1048,7 @@ function validateEventListenerOptions(options) {
 
   if (options === null)
     return kEmptyObject;
-  validateObject(options, 'options', {
-    allowArray: true, allowFunction: true,
-  });
+  validateObject(options, 'options', kValidateObjectAllowArray | kValidateObjectAllowFunction);
   return {
     once: Boolean(options.once),
     capture: Boolean(options.capture),
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -82,6 +82,7 @@ const {
   validateInteger,
   validateObject,
   validateString,
+  kValidateObjectAllowNullable,
 } = require('internal/validators');
 const pathModule = require('path');
 const {
@@ -596,7 +597,7 @@ async function read(handle, bufferOrParams, offset, length, position) {
   if (!isArrayBufferView(buffer)) {
     // This is fh.read(params)
     if (bufferOrParams !== undefined) {
-      validateObject(bufferOrParams, 'options', { nullable: true });
+      validateObject(bufferOrParams, 'options', kValidateObjectAllowNullable);
     }
     ({
       buffer = Buffer.alloc(16384),
diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
@@ -154,6 +154,7 @@ const { BuiltinModule } = require('internal/bootstrap/realm');
 const {
   validateObject,
   validateString,
+  kValidateObjectAllowArray,
 } = require('internal/validators');
 
 let hexSlice;
@@ -2155,7 +2156,7 @@ function format(...args) {
 }
 
 function formatWithOptions(inspectOptions, ...args) {
-  validateObject(inspectOptions, 'inspectOptions', { allowArray: true });
+  validateObject(inspectOptions, 'inspectOptions', kValidateObjectAllowArray);
   return formatWithOptionsInternal(inspectOptions, args);
 }
 
diff --git a/lib/internal/validators.js b/lib/internal/validators.js
@@ -177,7 +177,7 @@ function validateNumber(value, name, min = undefined, max) {
     throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
 
   if ((min != null && value < min) || (max != null && value > max) ||
-      ((min != null || max != null) && NumberIsNaN(value))) {
+    ((min != null || max != null) && NumberIsNaN(value))) {
     throw new ERR_OUT_OF_RANGE(
       name,
       `${min != null ? `>= ${min}` : ''}${min != null && max != null ? ' && ' : ''}${max != null ? `<= ${max}` : ''}`,
@@ -218,41 +218,48 @@ function validateBoolean(value, name) {
     throw new ERR_INVALID_ARG_TYPE(name, 'boolean', value);
 }
 
-/**
- * @param {any} options
- * @param {string} key
- * @param {boolean} defaultValue
- * @returns {boolean}
- */
-function getOwnPropertyValueOrDefault(options, key, defaultValue) {
-  return options == null || !ObjectPrototypeHasOwnProperty(options, key) ?
-    defaultValue :
-    options[key];
-}
+const kValidateObjectNone = 0;
+const kValidateObjectAllowNullable = 1 << 0;
+const kValidateObjectAllowArray = 1 << 1;
+const kValidateObjectAllowFunction = 1 << 2;
 
 /**
  * @callback validateObject
  * @param {*} value
  * @param {string} name
- * @param {{
- *   allowArray?: boolean,
- *   allowFunction?: boolean,
- *   nullable?: boolean
- * }} [options]
+ * @param {number} [options]
  */
 
 /** @type {validateObject} */
 const validateObject = hideStackFrames(
-  (value, name, options = null) => {
-    const allowArray = getOwnPropertyValueOrDefault(options, 'allowArray', false);
-    const allowFunction = getOwnPropertyValueOrDefault(options, 'allowFunction', false);
-    const nullable = getOwnPropertyValueOrDefault(options, 'nullable', false);
-    if ((!nullable && value === null) ||
-        (!allowArray && ArrayIsArray(value)) ||
-        (typeof value !== 'object' && (
-          !allowFunction || typeof value !== 'function'
-        ))) {
-      throw new ERR_INVALID_ARG_TYPE(name, 'Object', value);
+  (value, name, options = kValidateObjectNone) => {
+    if (options === kValidateObjectNone) {
+      if (value === null || ArrayIsArray(value)) {
+        throw new ERR_INVALID_ARG_TYPE(name, 'Object', value);
+      }
+
+      if (typeof value !== 'object') {
+        throw new ERR_INVALID_ARG_TYPE(name, 'Object', value);
+      }
+    } else {
+      const throwOnNullable = (kValidateObjectAllowNullable & options) === 0;
+
+      if (throwOnNullable && value === null) {
+        throw new ERR_INVALID_ARG_TYPE(name, 'Object', value);
+      }
+
+      const throwOnArray = (kValidateObjectAllowArray & options) === 0;
+
+      if (throwOnArray && ArrayIsArray(value)) {
+        throw new ERR_INVALID_ARG_TYPE(name, 'Object', value);
+      }
+
+      const throwOnFunction = (kValidateObjectAllowFunction & options) === 0;
+      const typeofValue = typeof value;
+
+      if (typeofValue !== 'object' && (throwOnFunction || typeofValue !== 'function')) {
+        throw new ERR_INVALID_ARG_TYPE(name, 'Object', value);
+      }
     }
   });
 
@@ -564,6 +571,10 @@ module.exports = {
   validateInteger,
   validateNumber,
   validateObject,
+  kValidateObjectNone,
+  kValidateObjectAllowNullable,
+  kValidateObjectAllowArray,
+  kValidateObjectAllowFunction,
   validateOneOf,
   validatePlainFunction,
   validatePort,
diff --git a/lib/internal/vm.js b/lib/internal/vm.js
@@ -17,13 +17,15 @@ const {
   validateString,
   validateStringArray,
   validateUint32,
+  kValidateObjectAllowArray,
+  kValidateObjectAllowNullable,
 } = require('internal/validators');
 const {
   ERR_INVALID_ARG_TYPE,
 } = require('internal/errors').codes;
 
 function isContext(object) {
-  validateObject(object, 'object', { __proto__: null, allowArray: true });
+  validateObject(object, 'object', kValidateObjectAllowArray);
 
   return _isContext(object);
 }
@@ -67,7 +69,7 @@ function internalCompileFunction(code, params, options) {
   validateArray(contextExtensions, 'options.contextExtensions');
   ArrayPrototypeForEach(contextExtensions, (extension, i) => {
     const name = `options.contextExtensions[${i}]`;
-    validateObject(extension, name, { __proto__: null, nullable: true });
+    validateObject(extension, name, kValidateObjectAllowNullable);
   });
 
   const result = compileFunction(
diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js
diff --git a/test/benchmark/test-benchmark-validators.js b/test/benchmark/test-benchmark-validators.js
diff --git a/test/parallel/test-validators.js b/test/parallel/test-validators.js

Original file line number	Diff line number	Diff line change
`@@ -154,6 +154,7 @@ const { BuiltinModule } = require('internal/bootstrap/realm');`
`154`	`154`	`const {`
`155`	`155`	`validateObject,`
`156`	`156`	`validateString,`
	`157`	`+ kValidateObjectAllowArray,`
`157`	`158`	`} = require('internal/validators');`
`158`	`159`
`159`	`160`	`let hexSlice;`
`@@ -2155,7 +2156,7 @@ function format(...args) {`
`2155`	`2156`	`}`
`2156`	`2157`
`2157`	`2158`	`function formatWithOptions(inspectOptions, ...args) {`
`2158`		`- validateObject(inspectOptions, 'inspectOptions', { allowArray: true });`
	`2159`	`+ validateObject(inspectOptions, 'inspectOptions', kValidateObjectAllowArray);`
`2159`	`2160`	`return formatWithOptionsInternal(inspectOptions, args);`
`2160`	`2161`	`}`
`2161`	`2162`