From 56b49e90afe7e581ecec85df40ffa97553046471 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 15 Dec 2025 13:27:40 +0100 Subject: [PATCH 1/3] assert,util: improve deep comparison performance --- lib/internal/util/comparisons.js | 114 ++++++++---------- .../test-assert-typedarray-deepequal.js | 46 ++++++- 2 files changed, 89 insertions(+), 71 deletions(-) diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js index cfbf393e0ba1dc..b58b828a96dcea 100644 --- a/lib/internal/util/comparisons.js +++ b/lib/internal/util/comparisons.js @@ -4,7 +4,6 @@ const { Array, ArrayBuffer, ArrayIsArray, - ArrayPrototypeFilter, ArrayPrototypePush, BigInt, BigInt64Array, @@ -91,7 +90,7 @@ const wellKnownConstructors = new SafeSet() .add(WeakMap) .add(WeakSet); -if (Float16Array) { // TODO(BridgeAR): Remove when regularly supported +if (Float16Array) { // TODO(BridgeAR): Remove when Flag got removed from V8 wellKnownConstructors.add(Float16Array); } @@ -127,6 +126,11 @@ const { getOwnNonIndexProperties, } = internalBinding('util'); +let kKeyObject; +let kExtractable; +let kAlgorithm; +let kKeyUsages; + const kStrict = 2; const kStrictWithoutPrototypes = 3; const kLoose = 0; @@ -152,7 +156,7 @@ function isPartialUint8Array(a, b) { } let offsetA = 0; for (let offsetB = 0; offsetB < lenB; offsetB++) { - while (!ObjectIs(a[offsetA], b[offsetB])) { + while (a[offsetA] !== b[offsetB]) { offsetA++; if (offsetA > lenA - lenB + offsetB) { return false; @@ -187,11 +191,7 @@ function areSimilarFloatArrays(a, b) { } function areSimilarTypedArrays(a, b) { - if (a.byteLength !== b.byteLength) { - return false; - } - return compare(new Uint8Array(a.buffer, a.byteOffset, a.byteLength), - new Uint8Array(b.buffer, b.byteOffset, b.byteLength)) === 0; + return a.byteLength === b.byteLength && compare(a, b) === 0; } function areEqualArrayBuffers(buf1, buf2) { @@ -225,7 +225,7 @@ function isEqualBoxedPrimitive(val1, val2) { assert.fail(`Unknown boxed type ${val1}`); } -function isEnumerableOrIdentical(val1, val2, prop, mode, memos, method) { +function isEnumerableOrIdentical(val1, val2, prop, mode, memos) { return hasEnumerable(val2, prop) || // This is handled by Object.keys() (mode === kPartial && (val2[prop] === undefined || (prop === 'message' && val2[prop] === ''))) || innerDeepEqual(val1[prop], val2[prop], mode, memos); @@ -400,8 +400,10 @@ function objectComparisonStart(val1, val2, mode, memos) { return false; } } else if (isCryptoKey(val1)) { - const { kKeyObject } = require('internal/crypto/util'); - const { kExtractable, kAlgorithm, kKeyUsages } = require('internal/crypto/keys'); + if (kKeyObject === undefined) { + kKeyObject = require('internal/crypto/util').kKeyObject; + ({ kExtractable, kAlgorithm, kKeyUsages } = require('internal/crypto/keys')); + } if (!isCryptoKey(val2) || val1[kExtractable] !== val2[kExtractable] || !innerDeepEqual(val1[kAlgorithm], val2[kAlgorithm], mode, memos) || @@ -417,18 +419,11 @@ function objectComparisonStart(val1, val2, mode, memos) { return keyCheck(val1, val2, mode, memos, kNoIterator); } -function getEnumerables(val, keys) { - return ArrayPrototypeFilter(keys, (key) => hasEnumerable(val, key)); -} - function partialSymbolEquiv(val1, val2, keys2) { const symbolKeys = getOwnSymbols(val2); if (symbolKeys.length !== 0) { for (const key of symbolKeys) { if (hasEnumerable(val2, key)) { - if (!hasEnumerable(val1, key)) { - return false; - } ArrayPrototypePush(keys2, key); } } @@ -460,32 +455,19 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) { } else if (keys2.length !== (keys1 = ObjectKeys(val1)).length) { return false; } else if (mode === kStrict || mode === kStrictWithoutPrototypes) { - const symbolKeysA = getOwnSymbols(val1); - if (symbolKeysA.length !== 0) { - let count = 0; - for (const key of symbolKeysA) { - if (hasEnumerable(val1, key)) { - if (!hasEnumerable(val2, key)) { - return false; - } - ArrayPrototypePush(keys2, key); - count++; - } else if (hasEnumerable(val2, key)) { - return false; - } - } - const symbolKeysB = getOwnSymbols(val2); - if (symbolKeysA.length !== symbolKeysB.length && - getEnumerables(val2, symbolKeysB).length !== count) { - return false; + for (const key of getOwnSymbols(val1)) { + if (hasEnumerable(val1, key)) { + ArrayPrototypePush(keys1, key); } - } else { - const symbolKeysB = getOwnSymbols(val2); - if (symbolKeysB.length !== 0 && - getEnumerables(val2, symbolKeysB).length !== 0) { - return false; + } + for (const key of getOwnSymbols(val2)) { + if (hasEnumerable(val2, key)) { + ArrayPrototypePush(keys2, key); } } + if (keys1.length !== keys2.length) { + return false; + } } } @@ -647,16 +629,14 @@ function partialObjectSetEquiv(array, a, b, mode, memo) { } function arrayHasEqualElement(array, val1, mode, memo, comparator, start, end) { - let matched = false; for (let i = end - 1; i >= start; i--) { if (comparator(val1, array[i], mode, memo)) { - // Remove the matching element to make sure we do not check that again. - array.splice(i, 1); - matched = true; - break; + // Move the matching element to make sure we do not check that again. + array[i] = array[end]; + return true; } } - return matched; + return false; } function setObjectEquiv(array, a, b, mode, memo) { @@ -798,18 +778,16 @@ function partialObjectMapEquiv(array, a, b, mode, memo) { } function arrayHasEqualMapElement(array, key1, item1, b, mode, memo, comparator, start, end) { - let matched = false; for (let i = end - 1; i >= start; i--) { const key2 = array[i]; if (comparator(key1, key2, mode, memo) && innerDeepEqual(item1, b.get(key2), mode, memo)) { - // Remove the matching element to make sure we do not check that again. - array.splice(i, 1); - matched = true; - break; + // Move the matching element to make sure we do not check that again. + array[i] = array[end]; + return true; } } - return matched; + return false; } function mapObjectEquiv(array, a, b, mode, memo) { @@ -898,17 +876,21 @@ function mapEquiv(a, b, mode, memo) { } function partialSparseArrayEquiv(a, b, mode, memos, startA, startB) { - let aPos = 0; - const keysA = ObjectKeys(a).slice(startA); - const keysB = ObjectKeys(b).slice(startB); - if (keysA.length < keysB.length) { + let aPos = startA; + const keysA = ObjectKeys(a); + const keysB = ObjectKeys(b); + const keysBLength = keysB.length; + const keysALength = keysA.length; + const lenA = keysALength - startA; + const lenB = keysBLength - startB; + if (lenA < lenB) { return false; } - for (let i = 0; i < keysB.length; i++) { - const keyB = keysB[i]; + for (let i = 0; i < lenB; i++) { + const keyB = keysB[startB + i]; while (!innerDeepEqual(a[keysA[aPos]], b[keyB], mode, memos)) { aPos++; - if (aPos > keysA.length - keysB.length + i) { + if (aPos > keysALength - lenB + i) { return false; } } @@ -979,8 +961,11 @@ function objEquiv(a, b, mode, keys1, keys2, memos, iterationType) { // property in V8 13.0 compared to calling Object.propertyIsEnumerable() // and accessing the property regularly. const descriptor = ObjectGetOwnPropertyDescriptor(a, key); - if (!descriptor?.enumerable || - !innerDeepEqual(descriptor.value !== undefined ? descriptor.value : a[key], b[key], mode, memos)) { + if (descriptor === undefined || descriptor.enumerable !== true) { + return false; + } + const value = descriptor.writable !== undefined ? descriptor.value : a[key]; + if (!innerDeepEqual(value, b[key], mode, memos)) { return false; } } @@ -1029,10 +1014,7 @@ module.exports = { return detectCycles(val1, val2, kLoose); }, isDeepStrictEqual(val1, val2, skipPrototype) { - if (skipPrototype) { - return detectCycles(val1, val2, kStrictWithoutPrototypes); - } - return detectCycles(val1, val2, kStrict); + return detectCycles(val1, val2, skipPrototype ? kStrictWithoutPrototypes : kStrict); }, isPartialStrictEqual(val1, val2) { return detectCycles(val1, val2, kPartial); diff --git a/test/parallel/test-assert-typedarray-deepequal.js b/test/parallel/test-assert-typedarray-deepequal.js index 2e222acdbd62ce..0ca174ea96c3db 100644 --- a/test/parallel/test-assert-typedarray-deepequal.js +++ b/test/parallel/test-assert-typedarray-deepequal.js @@ -35,7 +35,12 @@ suite('equalArrayPairs', () => { test('', () => { // eslint-disable-next-line no-restricted-properties assert.deepEqual(arrayPair[0], arrayPair[1]); + // eslint-disable-next-line no-restricted-properties + assert.deepEqual(arrayPair[1], arrayPair[0]); assert.deepStrictEqual(arrayPair[0], arrayPair[1]); + assert.deepStrictEqual(arrayPair[1], arrayPair[0]); + assert.partialDeepStrictEqual(arrayPair[0], arrayPair[1]); + assert.partialDeepStrictEqual(arrayPair[1], arrayPair[0]); }); } }); @@ -51,10 +56,24 @@ suite('looseEqualArrayPairs', () => { test('', () => { // eslint-disable-next-line no-restricted-properties assert.deepEqual(arrayPair[0], arrayPair[1]); + // eslint-disable-next-line no-restricted-properties + assert.deepEqual(arrayPair[1], arrayPair[0]); assert.throws( makeBlock(assert.deepStrictEqual, arrayPair[0], arrayPair[1]), assert.AssertionError ); + assert.throws( + makeBlock(assert.deepStrictEqual, arrayPair[1], arrayPair[0]), + assert.AssertionError + ); + assert.throws( + makeBlock(assert.partialDeepStrictEqual, arrayPair[0], arrayPair[1]), + assert.AssertionError + ); + assert.throws( + makeBlock(assert.partialDeepStrictEqual, arrayPair[1], arrayPair[0]), + assert.AssertionError + ); }); } }); @@ -65,7 +84,7 @@ suite('notEqualArrayPairs', () => { [new Int16Array(256), new Uint16Array(256)], [new Int16Array([256]), new Uint16Array([256])], [new Float64Array([+0.0]), new Float32Array([-0.0])], - [new Uint8Array(2), new Uint8Array(3)], + [new Uint8Array(2), new Uint8Array(3), 'unequal length'], [new Uint8Array([1, 2, 3]), new Uint8Array([4, 5, 6])], [new Uint8ClampedArray([300, 2, 3]), new Uint8Array([300, 2, 3])], [new Uint16Array([2]), new Uint16Array([3])], @@ -74,17 +93,17 @@ suite('notEqualArrayPairs', () => { [new Int16Array([-256]), new Uint16Array([0xff00])], // same bits [new Int32Array([-256]), new Uint32Array([0xffffff00])], // ditto [new Float16Array([0.1]), new Float16Array([0.0])], - [new Float16Array([0.1]), new Float16Array([0.1, 0.2])], + [new Float16Array([0.1]), new Float16Array([0.1, 0.2]), 'unequal length'], [new Float32Array([0.1]), new Float32Array([0.0])], - [new Float32Array([0.1]), new Float32Array([0.1, 0.2])], + [new Float32Array([0.1]), new Float32Array([0.1, 0.2]), 'unequal length'], [new Float64Array([0.1]), new Float64Array([0.0])], [new Uint8Array([1, 2, 3]).buffer, new Uint8Array([4, 5, 6]).buffer], [ new Uint8Array(new SharedArrayBuffer(3)).fill(1).buffer, new Uint8Array(new SharedArrayBuffer(3)).fill(2).buffer, ], - [new ArrayBuffer(2), new ArrayBuffer(3)], - [new SharedArrayBuffer(2), new SharedArrayBuffer(3)], + [new ArrayBuffer(2), new ArrayBuffer(3), 'unequal length'], + [new SharedArrayBuffer(2), new SharedArrayBuffer(3), 'unequal length'], [new ArrayBuffer(2), new SharedArrayBuffer(3)], [ new Uint8Array(new ArrayBuffer(3)).fill(1).buffer, @@ -101,14 +120,31 @@ suite('notEqualArrayPairs', () => { makeBlock(assert.deepEqual, arrayPair[0], arrayPair[1]), assert.AssertionError ); + assert.throws( + // eslint-disable-next-line no-restricted-properties + makeBlock(assert.deepEqual, arrayPair[1], arrayPair[0]), + assert.AssertionError + ); assert.throws( makeBlock(assert.deepStrictEqual, arrayPair[0], arrayPair[1]), assert.AssertionError ); + assert.throws( + makeBlock(assert.deepStrictEqual, arrayPair[1], arrayPair[0]), + assert.AssertionError + ); assert.throws( makeBlock(assert.partialDeepStrictEqual, arrayPair[0], arrayPair[1]), assert.AssertionError ); + if (arrayPair[2]) { + assert.partialDeepStrictEqual(arrayPair[1], arrayPair[0]); + } else { + assert.throws( + makeBlock(assert.partialDeepStrictEqual, arrayPair[1], arrayPair[0]), + assert.AssertionError + ); + } }); } }); From 9498747eb65e6ee51d024d4a06bf0cb02239261a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 15 Dec 2025 17:56:36 +0100 Subject: [PATCH 2/3] assert,util: fix deep comparing invalid dates skipping properties The property comparison of invalid dates regressed when starting to handle invalid dates as being equal. --- lib/internal/util/comparisons.js | 6 +++--- test/parallel/test-assert-deep.js | 13 ++++++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js index b58b828a96dcea..3b800b48b7c5d5 100644 --- a/lib/internal/util/comparisons.js +++ b/lib/internal/util/comparisons.js @@ -304,9 +304,9 @@ function objectComparisonStart(val1, val2, mode, memos) { } const time1 = DatePrototypeGetTime(val1); const time2 = DatePrototypeGetTime(val2); - if (time1 !== time2) { - // eslint-disable-next-line no-self-compare - return time1 !== time1 && time2 !== time2; + // eslint-disable-next-line no-self-compare + if (time1 !== time2 && (time1 === time1 || time2 === time2)) { + return false; } } else if (isRegExp(val1)) { if (!isRegExp(val2) || !areSimilarRegExps(val1, val2)) { diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js index 73da19017a8cc3..9c1d8eefe09252 100644 --- a/test/parallel/test-assert-deep.js +++ b/test/parallel/test-assert-deep.js @@ -752,7 +752,18 @@ test('Additional tests', () => { assertNotDeepOrStrict(new Date(), new Date(2000, 3, 14)); - assertDeepAndStrictEqual(new Date('foo'), new Date('bar')); + { + // Invalid dates deep comparison. + const date1 = new Date('foo'); + const date2 = new Date('bar'); + date1.foo = true; + date2.foo = true; + assertDeepAndStrictEqual(date1, date2); + + date1.bar = false; + date2.bar = true; + assertNotDeepOrStrict(date1, date2); + } assertDeepAndStrictEqual(/a/, /a/); assertDeepAndStrictEqual(/a/g, /a/g); From 645d0565effa241ba7f296ff243ea6a5169b9c7c Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 15 Dec 2025 18:18:53 +0100 Subject: [PATCH 3/3] assert: use a set instead of an array for faster lookup This is a very small improvement for error creation. --- lib/internal/assert/assertion_error.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js index 5c15b96b12d1ea..5dbf1e7a341380 100644 --- a/lib/internal/assert/assertion_error.js +++ b/lib/internal/assert/assertion_error.js @@ -10,6 +10,7 @@ const { ObjectDefineProperty, ObjectGetPrototypeOf, ObjectPrototypeHasOwnProperty, + SafeSet, String, StringPrototypeRepeat, StringPrototypeSlice, @@ -42,7 +43,10 @@ const kReadableOperator = { const kMaxShortStringLength = 12; const kMaxLongStringLength = 512; -const kMethodsWithCustomMessageDiff = ['deepStrictEqual', 'strictEqual', 'partialDeepStrictEqual']; +const kMethodsWithCustomMessageDiff = new SafeSet() + .add('deepStrictEqual') + .add('strictEqual') + .add('partialDeepStrictEqual'); function copyError(source) { const target = ObjectAssign( @@ -263,7 +267,7 @@ class AssertionError extends Error { if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0; if (message != null) { - if (kMethodsWithCustomMessageDiff.includes(operator)) { + if (kMethodsWithCustomMessageDiff.has(operator)) { super(createErrDiff(actual, expected, operator, message, diff)); } else { super(String(message)); @@ -283,7 +287,7 @@ class AssertionError extends Error { expected = copyError(expected); } - if (kMethodsWithCustomMessageDiff.includes(operator)) { + if (kMethodsWithCustomMessageDiff.has(operator)) { super(createErrDiff(actual, expected, operator, message, diff)); } else if (operator === 'notDeepStrictEqual' || operator === 'notStrictEqual') {