Skip to content

Commit

Permalink
lib: reduce overhead of validateObject
Browse files Browse the repository at this point in the history
PR-URL: nodejs#49928
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
H4ad authored and alexfernandez committed Nov 1, 2023
1 parent d1e7987 commit 89e547a
Show file tree
Hide file tree
Showing 12 changed files with 170 additions and 74 deletions.
75 changes: 75 additions & 0 deletions benchmark/validators/validate-object.js
Original file line number Diff line number Diff line change
@@ -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);
}
7 changes: 4 additions & 3 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ const {
validateInteger,
validateObject,
validateString,
kValidateObjectAllowNullable,
} = require('internal/validators');

let truncateWarn = true;
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand Down Expand Up @@ -715,7 +716,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);
}

({
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/abort_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ const {
validateAbortSignalArray,
validateObject,
validateUint32,
kValidateObjectAllowArray,
kValidateObjectAllowFunction,
} = require('internal/validators');

const {
Expand Down Expand Up @@ -436,7 +438,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();
Expand Down
31 changes: 11 additions & 20 deletions lib/internal/encoding.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ const {
const {
validateString,
validateObject,
kValidateObjectAllowNullable,
kValidateObjectAllowArray,
kValidateObjectAllowFunction,
} = require('internal/validators');
const binding = internalBinding('encoding_binding');
const {
Expand Down Expand Up @@ -390,6 +393,10 @@ const TextDecoder =
makeTextDecoderICU() :
makeTextDecoderJS();

const kValidateObjectAllowObjectsAndNull = kValidateObjectAllowNullable |
kValidateObjectAllowArray |
kValidateObjectAllowFunction;

function makeTextDecoderICU() {
const {
decode: _decode,
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 9 additions & 4 deletions lib/internal/event_target.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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,
Expand Down Expand Up @@ -1037,9 +1044,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),
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ const {
validateInteger,
validateObject,
validateString,
kValidateObjectAllowNullable,
} = require('internal/validators');
const pathModule = require('path');
const {
Expand Down Expand Up @@ -597,7 +598,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),
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ const { BuiltinModule } = require('internal/bootstrap/realm');
const {
validateObject,
validateString,
kValidateObjectAllowArray,
} = require('internal/validators');

let hexSlice;
Expand Down Expand Up @@ -2155,7 +2156,7 @@ function format(...args) {
}

function formatWithOptions(inspectOptions, ...args) {
validateObject(inspectOptions, 'inspectOptions', { allowArray: true });
validateObject(inspectOptions, 'inspectOptions', kValidateObjectAllowArray);
return formatWithOptionsInternal(inspectOptions, args);
}

Expand Down
65 changes: 38 additions & 27 deletions lib/internal/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}` : ''}`,
Expand Down Expand Up @@ -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);
}
}
});

Expand Down Expand Up @@ -564,6 +571,10 @@ module.exports = {
validateInteger,
validateNumber,
validateObject,
kValidateObjectNone,
kValidateObjectAllowNullable,
kValidateObjectAllowArray,
kValidateObjectAllowFunction,
validateOneOf,
validatePlainFunction,
validatePort,
Expand Down
6 changes: 4 additions & 2 deletions lib/internal/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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(
Expand Down
Loading

0 comments on commit 89e547a

Please sign in to comment.