assert,util: improve deep equal comparison performance

This is mainly a performance improvement for a lot of simple cases.
Diverging elements are detected earlier and equal entries are
partially also detected faster.

A small correctness patch is also included where recursions now
stop as soon as either side has a circular structure. Before, both
sides had to have a circular structure at the specific comparison
which could have caused more checks that likely fail at a later
point.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: https://github.com/nodejs/node/pull/46593
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
Ruben Bridgewater 2023-02-09 22:12:25 +01:00 committed by Antoine du Hamel
parent a02b434966
commit 449e9f4489
4 changed files with 176 additions and 133 deletions

View File

@ -12,7 +12,8 @@ const bench = common.createBenchmark(main, {
combinationFilter: (p) => {
return p.size === 1e4 && p.n === 25 ||
p.size === 1e3 && p.n === 2e2 ||
p.size === 1e2 && p.n === 2e3;
p.size === 1e2 && p.n === 2e3 ||
p.size === 1;
},
});

View File

@ -546,6 +546,10 @@ An alias of [`assert.ok()`][].
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/46593
description: Recursion now stops when either side encounters a circular
reference.
- version: v18.0.0
pr-url: https://github.com/nodejs/node/pull/41020
description: Regular expressions lastIndex property is now compared as well.
@ -617,7 +621,7 @@ are also recursively evaluated by the following rules.
* [Object wrappers][] are compared both as objects and unwrapped values.
* `Object` properties are compared unordered.
* [`Map`][] keys and [`Set`][] items are compared unordered.
* Recursion stops when both sides differ or both sides encounter a circular
* Recursion stops when both sides differ or either side encounters a circular
reference.
* Implementation does not test the [`[[Prototype]]`][prototype-spec] of
objects.
@ -727,6 +731,10 @@ parameter is an instance of an [`Error`][] then it will be thrown instead of the
<!-- YAML
added: v1.2.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/46593
description: Recursion now stops when either side encounters a circular
reference.
- version: v18.0.0
pr-url: https://github.com/nodejs/node/pull/41020
description: Regular expressions lastIndex property is now compared as well.
@ -780,7 +788,7 @@ are recursively evaluated also by the following rules.
* [Object wrappers][] are compared both as objects and unwrapped values.
* `Object` properties are compared unordered.
* [`Map`][] keys and [`Set`][] items are compared unordered.
* Recursion stops when both sides differ or both sides encounter a circular
* Recursion stops when both sides differ or either side encounters a circular
reference.
* [`WeakMap`][] and [`WeakSet`][] comparison does not rely on their values. See
below for further details.

View File

@ -17,7 +17,6 @@ const {
ObjectPrototypeHasOwnProperty,
ObjectPrototypePropertyIsEnumerable,
ObjectPrototypeToString,
SafeMap,
SafeSet,
StringPrototypeValueOf,
SymbolPrototypeValueOf,
@ -134,21 +133,19 @@ function isEqualBoxedPrimitive(val1, val2) {
function innerDeepEqual(val1, val2, strict, memos) {
// All identical values are equivalent, as determined by ===.
if (val1 === val2) {
if (val1 !== 0)
return true;
return strict ? ObjectIs(val1, val2) : true;
return val1 !== 0 || ObjectIs(val1, val2) || !strict;
}
// Check more closely if val1 and val2 are equal.
if (strict) {
if (typeof val1 !== 'object') {
return typeof val1 === 'number' && NumberIsNaN(val1) &&
NumberIsNaN(val2);
if (typeof val1 === 'number') {
return NumberIsNaN(val1) && NumberIsNaN(val2);
}
if (typeof val2 !== 'object' || val1 === null || val2 === null) {
return false;
}
if (ObjectGetPrototypeOf(val1) !== ObjectGetPrototypeOf(val2)) {
if (typeof val2 !== 'object' ||
typeof val1 !== 'object' ||
val1 === null ||
val2 === null ||
ObjectGetPrototypeOf(val1) !== ObjectGetPrototypeOf(val2)) {
return false;
}
} else {
@ -193,14 +190,6 @@ function innerDeepEqual(val1, val2, strict, memos) {
if (!isRegExp(val2) || !areSimilarRegExps(val1, val2)) {
return false;
}
} else if (isNativeError(val1) || val1 instanceof Error) {
// Do not compare the stack as it might differ even though the error itself
// is otherwise identical.
if ((!isNativeError(val2) && !(val2 instanceof Error)) ||
val1.message !== val2.message ||
val1.name !== val2.name) {
return false;
}
} else if (isArrayBufferView(val1)) {
if (TypedArrayPrototypeGetSymbolToStringTag(val1) !==
TypedArrayPrototypeGetSymbolToStringTag(val2)) {
@ -237,6 +226,14 @@ function innerDeepEqual(val1, val2, strict, memos) {
if (!isAnyArrayBuffer(val2) || !areEqualArrayBuffers(val1, val2)) {
return false;
}
} else if (isNativeError(val1) || val1 instanceof Error) {
// Do not compare the stack as it might differ even though the error itself
// is otherwise identical.
if ((!isNativeError(val2) && !(val2 instanceof Error)) ||
val1.message !== val2.message ||
val1.name !== val2.name) {
return false;
}
} else if (isBoxedPrimitive(val1)) {
if (!isEqualBoxedPrimitive(val1, val2)) {
return false;
@ -271,50 +268,53 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
// c) Equivalent values for every corresponding key/index
// d) For Sets and Maps, equal contents
// Note: this accounts for both named and indexed properties on Arrays.
if (arguments.length === 5) {
aKeys = ObjectKeys(val1);
const bKeys = ObjectKeys(val2);
const isArrayLikeObject = aKeys !== undefined;
// The pair must have the same number of owned properties.
if (aKeys.length !== bKeys.length) {
return false;
}
if (aKeys === undefined) {
aKeys = ObjectKeys(val1);
}
// Cheap key test
let i = 0;
for (; i < aKeys.length; i++) {
if (!ObjectPrototypePropertyIsEnumerable(val2, aKeys[i])) {
return false;
if (aKeys.length > 0) {
for (const key of aKeys) {
if (!ObjectPrototypePropertyIsEnumerable(val2, key)) {
return false;
}
}
}
if (strict && arguments.length === 5) {
const symbolKeysA = ObjectGetOwnPropertySymbols(val1);
if (symbolKeysA.length !== 0) {
let count = 0;
for (i = 0; i < symbolKeysA.length; i++) {
const key = symbolKeysA[i];
if (ObjectPrototypePropertyIsEnumerable(val1, key)) {
if (!ObjectPrototypePropertyIsEnumerable(val2, key)) {
if (!isArrayLikeObject) {
// The pair must have the same number of owned properties.
if (aKeys.length !== ObjectKeys(val2).length) {
return false;
}
if (strict) {
const symbolKeysA = ObjectGetOwnPropertySymbols(val1);
if (symbolKeysA.length !== 0) {
let count = 0;
for (const key of symbolKeysA) {
if (ObjectPrototypePropertyIsEnumerable(val1, key)) {
if (!ObjectPrototypePropertyIsEnumerable(val2, key)) {
return false;
}
ArrayPrototypePush(aKeys, key);
count++;
} else if (ObjectPrototypePropertyIsEnumerable(val2, key)) {
return false;
}
ArrayPrototypePush(aKeys, key);
count++;
} else if (ObjectPrototypePropertyIsEnumerable(val2, key)) {
}
const symbolKeysB = ObjectGetOwnPropertySymbols(val2);
if (symbolKeysA.length !== symbolKeysB.length &&
getEnumerables(val2, symbolKeysB).length !== count) {
return false;
}
} else {
const symbolKeysB = ObjectGetOwnPropertySymbols(val2);
if (symbolKeysB.length !== 0 &&
getEnumerables(val2, symbolKeysB).length !== 0) {
return false;
}
}
const symbolKeysB = ObjectGetOwnPropertySymbols(val2);
if (symbolKeysA.length !== symbolKeysB.length &&
getEnumerables(val2, symbolKeysB).length !== count) {
return false;
}
} else {
const symbolKeysB = ObjectGetOwnPropertySymbols(val2);
if (symbolKeysB.length !== 0 &&
getEnumerables(val2, symbolKeysB).length !== 0) {
return false;
}
}
}
@ -329,45 +329,74 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
// Use memos to handle cycles.
if (memos === undefined) {
memos = {
val1: new SafeMap(),
val2: new SafeMap(),
position: 0
set: undefined,
a: val1,
b: val2,
c: undefined,
d: undefined,
deep: false,
deleteFailures: false,
};
} else {
// We prevent up to two map.has(x) calls by directly retrieving the value
// and checking for undefined. The map can only contain numbers, so it is
// safe to check for undefined only.
const val2MemoA = memos.val1.get(val1);
if (val2MemoA !== undefined) {
const val2MemoB = memos.val2.get(val2);
if (val2MemoB !== undefined) {
return val2MemoA === val2MemoB;
}
}
memos.position++;
return objEquiv(val1, val2, strict, aKeys, memos, iterationType);
}
memos.val1.set(val1, memos.position);
memos.val2.set(val2, memos.position);
if (memos.set === undefined) {
if (memos.deep === false) {
if (memos.a === val1) {
return memos.b === val2;
}
if (memos.b === val2) {
return false;
}
memos.c = val1;
memos.d = val2;
memos.deep = true;
const result = objEquiv(val1, val2, strict, aKeys, memos, iterationType);
memos.deep = false;
return result;
}
memos.set = new SafeSet();
memos.set.add(memos.a);
memos.set.add(memos.b);
memos.set.add(memos.c);
memos.set.add(memos.d);
}
const { set } = memos;
const originalSize = set.size;
set.add(val1);
if (originalSize === set.size) {
return set.has(val2);
}
set.add(val2);
if (originalSize === set.size - 1) {
return false;
}
const areEq = objEquiv(val1, val2, strict, aKeys, memos, iterationType);
memos.val1.delete(val1);
memos.val2.delete(val2);
if (areEq || memos.deleteFailures) {
set.delete(val1);
set.delete(val2);
}
return areEq;
}
function setHasEqualElement(set, val1, strict, memo) {
// Go looking.
for (const val2 of set) {
function setHasEqualElement(set, val2, strict, memo) {
const { deleteFailures } = memo;
memo.deleteFailures = true;
for (const val1 of set) {
if (innerDeepEqual(val1, val2, strict, memo)) {
// Remove the matching element to make sure we do not check that again.
set.delete(val2);
set.delete(val1);
memo.deleteFailures = deleteFailures;
return true;
}
}
memo.deleteFailures = deleteFailures;
return false;
}
@ -422,38 +451,28 @@ function setEquiv(a, b, strict, memo) {
// pairwise.
let set = null;
for (const val of a) {
// Note: Checking for the objects first improves the performance for object
// heavy sets but it is a minor slow down for primitives. As they are fast
// to check this improves the worst case scenario instead.
if (typeof val === 'object' && val !== null) {
if (set === null) {
set = new SafeSet();
}
// If the specified value doesn't exist in the second set it's a non-null
// object (or non strict only: a not matching primitive) we'll need to go
// hunting for something that's deep-(strict-)equal to it. To make this
// O(n log n) complexity we have to copy these values in a new set first.
set.add(val);
} else if (!b.has(val)) {
if (strict)
return false;
// Fast path to detect missing string, symbol, undefined and null values.
if (!setMightHaveLoosePrim(a, b, val)) {
if (!b.has(val)) {
if ((typeof val !== 'object' || val === null) &&
(strict || !setMightHaveLoosePrim(a, b, val))) {
return false;
}
if (set === null) {
if (b.size === 1) {
return innerDeepEqual(val, b.values().next().value, strict, memo);
}
set = new SafeSet();
}
// If the specified value doesn't exist in the second set it's a object
// (or in loose mode: a non-matching primitive). Find the
// deep-(strict-)equal element in a set copy to reduce duplicate checks.
set.add(val);
}
}
if (set !== null) {
for (const val of b) {
// We have to check if a primitive value is already
// matching and only if it's not, go hunting for it.
// Primitive values have already been handled above.
if (typeof val === 'object' && val !== null) {
if (!setHasEqualElement(set, val, strict, memo))
return false;
@ -469,18 +488,22 @@ function setEquiv(a, b, strict, memo) {
return true;
}
function mapHasEqualEntry(set, map, key1, item1, strict, memo) {
function mapHasEqualEntry(set, map, key2, item2, strict, memo) {
// To be able to handle cases like:
// Map([[{}, 'a'], [{}, 'b']]) vs Map([[{}, 'b'], [{}, 'a']])
// ... we need to consider *all* matching keys, not just the first we find.
for (const key2 of set) {
const { deleteFailures } = memo;
memo.deleteFailures = true;
for (const key1 of set) {
if (innerDeepEqual(key1, key2, strict, memo) &&
innerDeepEqual(item1, map.get(key2), strict, memo)) {
set.delete(key2);
innerDeepEqual(map.get(key1), item2, strict, memo)) {
set.delete(key1);
memo.deleteFailures = deleteFailures;
return true;
}
}
memo.deleteFailures = deleteFailures;
return false;
}
@ -490,6 +513,11 @@ function mapEquiv(a, b, strict, memo) {
for (const { 0: key, 1: item1 } of a) {
if (typeof key === 'object' && key !== null) {
if (set === null) {
if (b.size === 1) {
const { 0: key2, 1: item2 } = b.entries().next().value;
return innerDeepEqual(key, key2, strict, memo) &&
innerDeepEqual(item1, item2, strict, memo);
}
set = new SafeSet();
}
set.add(key);
@ -520,8 +548,8 @@ function mapEquiv(a, b, strict, memo) {
return false;
} else if (!strict &&
(!a.has(key) ||
!innerDeepEqual(a.get(key), item, false, memo)) &&
!mapHasEqualEntry(set, a, key, item, false, memo)) {
!innerDeepEqual(a.get(key), item, strict, memo)) &&
!mapHasEqualEntry(set, a, key, item, strict, memo)) {
return false;
}
}
@ -532,20 +560,17 @@ function mapEquiv(a, b, strict, memo) {
}
function objEquiv(a, b, strict, keys, memos, iterationType) {
// Sets and maps don't have their entries accessible via normal object
// properties.
let i = 0;
// The pair must have equivalent values for every corresponding key.
if (keys.length > 0) {
for (const key of keys) {
if (!innerDeepEqual(a[key], b[key], strict, memos)) {
return false;
}
}
}
if (iterationType === kIsSet) {
if (!setEquiv(a, b, strict, memos)) {
return false;
}
} else if (iterationType === kIsMap) {
if (!mapEquiv(a, b, strict, memos)) {
return false;
}
} else if (iterationType === kIsArray) {
for (; i < a.length; i++) {
if (iterationType === kIsArray) {
for (let i = 0; i < a.length; i++) {
if (ObjectPrototypeHasOwnProperty(a, i)) {
if (!ObjectPrototypeHasOwnProperty(b, i) ||
!innerDeepEqual(a[i], b[i], strict, memos)) {
@ -563,22 +588,19 @@ function objEquiv(a, b, strict, keys, memos, iterationType) {
return false;
}
}
if (keysA.length !== ObjectKeys(b).length) {
return false;
}
return true;
return keysA.length === ObjectKeys(b).length;
}
}
}
// The pair must have equivalent values for every corresponding key.
// Possibly expensive deep test:
for (i = 0; i < keys.length; i++) {
const key = keys[i];
if (!innerDeepEqual(a[key], b[key], strict, memos)) {
} else if (iterationType === kIsSet) {
if (!setEquiv(a, b, strict, memos)) {
return false;
}
} else if (iterationType === kIsMap) {
if (!mapEquiv(a, b, strict, memos)) {
return false;
}
}
return true;
}

View File

@ -419,21 +419,33 @@ assertNotDeepOrStrict(
// GH-14441. Circular structures should be consistent
{
const a = {};
const b = {};
a.a = a;
const b = {};
b.a = {};
b.a.a = a;
assertDeepAndStrictEqual(a, b);
assertNotDeepOrStrict(a, b);
}
{
const a = {};
a.a = a;
const b = {};
b.a = b;
const c = {};
c.a = a;
assertNotDeepOrStrict(b, c);
}
{
const a = new Set();
const b = new Set();
const c = new Set();
a.add(a);
const b = new Set();
b.add(b);
const c = new Set();
c.add(a);
assertDeepAndStrictEqual(b, c);
assertNotDeepOrStrict(b, c);
}
// https://github.com/nodejs/node-v0.x-archive/pull/7178