Skip to content

Commit a968e4e

Browse files
BridgeARnodejs-github-bot
BridgeAR
authored and
nodejs-github-bot
committed
assert,util: improve deep comparison performance
PR-URL: #61076 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 416db75 commit a968e4e

File tree

2 files changed

+89
-71
lines changed

2 files changed

+89
-71
lines changed

‎lib/internal/util/comparisons.js‎

Lines changed: 48 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const {
44
Array,
55
ArrayBuffer,
66
ArrayIsArray,
7-
ArrayPrototypeFilter,
87
ArrayPrototypePush,
98
BigInt,
109
BigInt64Array,
@@ -91,7 +90,7 @@ const wellKnownConstructors = new SafeSet()
9190
.add(WeakMap)
9291
.add(WeakSet);
9392

94-
if (Float16Array) { // TODO(BridgeAR): Remove when regularly supported
93+
if (Float16Array) { // TODO(BridgeAR): Remove when Flag got removed from V8
9594
wellKnownConstructors.add(Float16Array);
9695
}
9796

@@ -127,6 +126,11 @@ const {
127126
getOwnNonIndexProperties,
128127
} = internalBinding('util');
129128

129+
let kKeyObject;
130+
let kExtractable;
131+
let kAlgorithm;
132+
let kKeyUsages;
133+
130134
const kStrict = 2;
131135
const kStrictWithoutPrototypes = 3;
132136
const kLoose = 0;
@@ -152,7 +156,7 @@ function isPartialUint8Array(a, b) {
152156
}
153157
let offsetA = 0;
154158
for (let offsetB = 0; offsetB < lenB; offsetB++) {
155-
while (!ObjectIs(a[offsetA], b[offsetB])) {
159+
while (a[offsetA] !== b[offsetB]) {
156160
offsetA++;
157161
if (offsetA > lenA - lenB + offsetB) {
158162
return false;
@@ -187,11 +191,7 @@ function areSimilarFloatArrays(a, b) {
187191
}
188192

189193
function areSimilarTypedArrays(a, b) {
190-
if (a.byteLength !== b.byteLength) {
191-
return false;
192-
}
193-
return compare(new Uint8Array(a.buffer, a.byteOffset, a.byteLength),
194-
new Uint8Array(b.buffer, b.byteOffset, b.byteLength)) === 0;
194+
return a.byteLength === b.byteLength && compare(a, b) === 0;
195195
}
196196

197197
function areEqualArrayBuffers(buf1, buf2) {
@@ -225,7 +225,7 @@ function isEqualBoxedPrimitive(val1, val2) {
225225
assert.fail(`Unknown boxed type ${val1}`);
226226
}
227227

228-
function isEnumerableOrIdentical(val1, val2, prop, mode, memos, method) {
228+
function isEnumerableOrIdentical(val1, val2, prop, mode, memos) {
229229
return hasEnumerable(val2, prop) || // This is handled by Object.keys()
230230
(mode === kPartial && (val2[prop] === undefined || (prop === 'message' && val2[prop] === ''))) ||
231231
innerDeepEqual(val1[prop], val2[prop], mode, memos);
@@ -400,8 +400,10 @@ function objectComparisonStart(val1, val2, mode, memos) {
400400
return false;
401401
}
402402
} else if (isCryptoKey(val1)) {
403-
const { kKeyObject } = require('internal/crypto/util');
404-
const { kExtractable, kAlgorithm, kKeyUsages } = require('internal/crypto/keys');
403+
if (kKeyObject === undefined) {
404+
kKeyObject = require('internal/crypto/util').kKeyObject;
405+
({ kExtractable, kAlgorithm, kKeyUsages } = require('internal/crypto/keys'));
406+
}
405407
if (!isCryptoKey(val2) ||
406408
val1[kExtractable] !== val2[kExtractable] ||
407409
!innerDeepEqual(val1[kAlgorithm], val2[kAlgorithm], mode, memos) ||
@@ -417,18 +419,11 @@ function objectComparisonStart(val1, val2, mode, memos) {
417419
return keyCheck(val1, val2, mode, memos, kNoIterator);
418420
}
419421

420-
function getEnumerables(val, keys) {
421-
return ArrayPrototypeFilter(keys, (key) => hasEnumerable(val, key));
422-
}
423-
424422
function partialSymbolEquiv(val1, val2, keys2) {
425423
const symbolKeys = getOwnSymbols(val2);
426424
if (symbolKeys.length !== 0) {
427425
for (const key of symbolKeys) {
428426
if (hasEnumerable(val2, key)) {
429-
if (!hasEnumerable(val1, key)) {
430-
return false;
431-
}
432427
ArrayPrototypePush(keys2, key);
433428
}
434429
}
@@ -460,32 +455,19 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
460455
} else if (keys2.length !== (keys1 = ObjectKeys(val1)).length) {
461456
return false;
462457
} else if (mode === kStrict || mode === kStrictWithoutPrototypes) {
463-
const symbolKeysA = getOwnSymbols(val1);
464-
if (symbolKeysA.length !== 0) {
465-
let count = 0;
466-
for (const key of symbolKeysA) {
467-
if (hasEnumerable(val1, key)) {
468-
if (!hasEnumerable(val2, key)) {
469-
return false;
470-
}
471-
ArrayPrototypePush(keys2, key);
472-
count++;
473-
} else if (hasEnumerable(val2, key)) {
474-
return false;
475-
}
476-
}
477-
const symbolKeysB = getOwnSymbols(val2);
478-
if (symbolKeysA.length !== symbolKeysB.length &&
479-
getEnumerables(val2, symbolKeysB).length !== count) {
480-
return false;
458+
for (const key of getOwnSymbols(val1)) {
459+
if (hasEnumerable(val1, key)) {
460+
ArrayPrototypePush(keys1, key);
481461
}
482-
} else {
483-
const symbolKeysB = getOwnSymbols(val2);
484-
if (symbolKeysB.length !== 0 &&
485-
getEnumerables(val2, symbolKeysB).length !== 0) {
486-
return false;
462+
}
463+
for (const key of getOwnSymbols(val2)) {
464+
if (hasEnumerable(val2, key)) {
465+
ArrayPrototypePush(keys2, key);
487466
}
488467
}
468+
if (keys1.length !== keys2.length) {
469+
return false;
470+
}
489471
}
490472
}
491473

@@ -647,16 +629,14 @@ function partialObjectSetEquiv(array, a, b, mode, memo) {
647629
}
648630

649631
function arrayHasEqualElement(array, val1, mode, memo, comparator, start, end) {
650-
let matched = false;
651632
for (let i = end - 1; i >= start; i--) {
652633
if (comparator(val1, array[i], mode, memo)) {
653-
// Remove the matching element to make sure we do not check that again.
654-
array.splice(i, 1);
655-
matched = true;
656-
break;
634+
// Move the matching element to make sure we do not check that again.
635+
array[i] = array[end];
636+
return true;
657637
}
658638
}
659-
return matched;
639+
return false;
660640
}
661641

662642
function setObjectEquiv(array, a, b, mode, memo) {
@@ -798,18 +778,16 @@ function partialObjectMapEquiv(array, a, b, mode, memo) {
798778
}
799779

800780
function arrayHasEqualMapElement(array, key1, item1, b, mode, memo, comparator, start, end) {
801-
let matched = false;
802781
for (let i = end - 1; i >= start; i--) {
803782
const key2 = array[i];
804783
if (comparator(key1, key2, mode, memo) &&
805784
innerDeepEqual(item1, b.get(key2), mode, memo)) {
806-
// Remove the matching element to make sure we do not check that again.
807-
array.splice(i, 1);
808-
matched = true;
809-
break;
785+
// Move the matching element to make sure we do not check that again.
786+
array[i] = array[end];
787+
return true;
810788
}
811789
}
812-
return matched;
790+
return false;
813791
}
814792

815793
function mapObjectEquiv(array, a, b, mode, memo) {
@@ -898,17 +876,21 @@ function mapEquiv(a, b, mode, memo) {
898876
}
899877

900878
function partialSparseArrayEquiv(a, b, mode, memos, startA, startB) {
901-
let aPos = 0;
902-
const keysA = ObjectKeys(a).slice(startA);
903-
const keysB = ObjectKeys(b).slice(startB);
904-
if (keysA.length < keysB.length) {
879+
let aPos = startA;
880+
const keysA = ObjectKeys(a);
881+
const keysB = ObjectKeys(b);
882+
const keysBLength = keysB.length;
883+
const keysALength = keysA.length;
884+
const lenA = keysALength - startA;
885+
const lenB = keysBLength - startB;
886+
if (lenA < lenB) {
905887
return false;
906888
}
907-
for (let i = 0; i < keysB.length; i++) {
908-
const keyB = keysB[i];
889+
for (let i = 0; i < lenB; i++) {
890+
const keyB = keysB[startB + i];
909891
while (!innerDeepEqual(a[keysA[aPos]], b[keyB], mode, memos)) {
910892
aPos++;
911-
if (aPos > keysA.length - keysB.length + i) {
893+
if (aPos > keysALength - lenB + i) {
912894
return false;
913895
}
914896
}
@@ -979,8 +961,11 @@ function objEquiv(a, b, mode, keys1, keys2, memos, iterationType) {
979961
// property in V8 13.0 compared to calling Object.propertyIsEnumerable()
980962
// and accessing the property regularly.
981963
const descriptor = ObjectGetOwnPropertyDescriptor(a, key);
982-
if (!descriptor?.enumerable ||
983-
!innerDeepEqual(descriptor.value !== undefined ? descriptor.value : a[key], b[key], mode, memos)) {
964+
if (descriptor === undefined || descriptor.enumerable !== true) {
965+
return false;
966+
}
967+
const value = descriptor.writable !== undefined ? descriptor.value : a[key];
968+
if (!innerDeepEqual(value, b[key], mode, memos)) {
984969
return false;
985970
}
986971
}
@@ -1029,10 +1014,7 @@ module.exports = {
10291014
return detectCycles(val1, val2, kLoose);
10301015
},
10311016
isDeepStrictEqual(val1, val2, skipPrototype) {
1032-
if (skipPrototype) {
1033-
return detectCycles(val1, val2, kStrictWithoutPrototypes);
1034-
}
1035-
return detectCycles(val1, val2, kStrict);
1017+
return detectCycles(val1, val2, skipPrototype ? kStrictWithoutPrototypes : kStrict);
10361018
},
10371019
isPartialStrictEqual(val1, val2) {
10381020
return detectCycles(val1, val2, kPartial);

‎test/parallel/test-assert-typedarray-deepequal.js‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,12 @@ suite('equalArrayPairs', () => {
3535
test('', () => {
3636
// eslint-disable-next-line no-restricted-properties
3737
assert.deepEqual(arrayPair[0], arrayPair[1]);
38+
// eslint-disable-next-line no-restricted-properties
39+
assert.deepEqual(arrayPair[1], arrayPair[0]);
3840
assert.deepStrictEqual(arrayPair[0], arrayPair[1]);
41+
assert.deepStrictEqual(arrayPair[1], arrayPair[0]);
42+
assert.partialDeepStrictEqual(arrayPair[0], arrayPair[1]);
43+
assert.partialDeepStrictEqual(arrayPair[1], arrayPair[0]);
3944
});
4045
}
4146
});
@@ -51,10 +56,24 @@ suite('looseEqualArrayPairs', () => {
5156
test('', () => {
5257
// eslint-disable-next-line no-restricted-properties
5358
assert.deepEqual(arrayPair[0], arrayPair[1]);
59+
// eslint-disable-next-line no-restricted-properties
60+
assert.deepEqual(arrayPair[1], arrayPair[0]);
5461
assert.throws(
5562
makeBlock(assert.deepStrictEqual, arrayPair[0], arrayPair[1]),
5663
assert.AssertionError
5764
);
65+
assert.throws(
66+
makeBlock(assert.deepStrictEqual, arrayPair[1], arrayPair[0]),
67+
assert.AssertionError
68+
);
69+
assert.throws(
70+
makeBlock(assert.partialDeepStrictEqual, arrayPair[0], arrayPair[1]),
71+
assert.AssertionError
72+
);
73+
assert.throws(
74+
makeBlock(assert.partialDeepStrictEqual, arrayPair[1], arrayPair[0]),
75+
assert.AssertionError
76+
);
5877
});
5978
}
6079
});
@@ -65,7 +84,7 @@ suite('notEqualArrayPairs', () => {
6584
[new Int16Array(256), new Uint16Array(256)],
6685
[new Int16Array([256]), new Uint16Array([256])],
6786
[new Float64Array([+0.0]), new Float32Array([-0.0])],
68-
[new Uint8Array(2), new Uint8Array(3)],
87+
[new Uint8Array(2), new Uint8Array(3), 'unequal length'],
6988
[new Uint8Array([1, 2, 3]), new Uint8Array([4, 5, 6])],
7089
[new Uint8ClampedArray([300, 2, 3]), new Uint8Array([300, 2, 3])],
7190
[new Uint16Array([2]), new Uint16Array([3])],
@@ -74,17 +93,17 @@ suite('notEqualArrayPairs', () => {
7493
[new Int16Array([-256]), new Uint16Array([0xff00])], // same bits
7594
[new Int32Array([-256]), new Uint32Array([0xffffff00])], // ditto
7695
[new Float16Array([0.1]), new Float16Array([0.0])],
77-
[new Float16Array([0.1]), new Float16Array([0.1, 0.2])],
96+
[new Float16Array([0.1]), new Float16Array([0.1, 0.2]), 'unequal length'],
7897
[new Float32Array([0.1]), new Float32Array([0.0])],
79-
[new Float32Array([0.1]), new Float32Array([0.1, 0.2])],
98+
[new Float32Array([0.1]), new Float32Array([0.1, 0.2]), 'unequal length'],
8099
[new Float64Array([0.1]), new Float64Array([0.0])],
81100
[new Uint8Array([1, 2, 3]).buffer, new Uint8Array([4, 5, 6]).buffer],
82101
[
83102
new Uint8Array(new SharedArrayBuffer(3)).fill(1).buffer,
84103
new Uint8Array(new SharedArrayBuffer(3)).fill(2).buffer,
85104
],
86-
[new ArrayBuffer(2), new ArrayBuffer(3)],
87-
[new SharedArrayBuffer(2), new SharedArrayBuffer(3)],
105+
[new ArrayBuffer(2), new ArrayBuffer(3), 'unequal length'],
106+
[new SharedArrayBuffer(2), new SharedArrayBuffer(3), 'unequal length'],
88107
[new ArrayBuffer(2), new SharedArrayBuffer(3)],
89108
[
90109
new Uint8Array(new ArrayBuffer(3)).fill(1).buffer,
@@ -101,14 +120,31 @@ suite('notEqualArrayPairs', () => {
101120
makeBlock(assert.deepEqual, arrayPair[0], arrayPair[1]),
102121
assert.AssertionError
103122
);
123+
assert.throws(
124+
// eslint-disable-next-line no-restricted-properties
125+
makeBlock(assert.deepEqual, arrayPair[1], arrayPair[0]),
126+
assert.AssertionError
127+
);
104128
assert.throws(
105129
makeBlock(assert.deepStrictEqual, arrayPair[0], arrayPair[1]),
106130
assert.AssertionError
107131
);
132+
assert.throws(
133+
makeBlock(assert.deepStrictEqual, arrayPair[1], arrayPair[0]),
134+
assert.AssertionError
135+
);
108136
assert.throws(
109137
makeBlock(assert.partialDeepStrictEqual, arrayPair[0], arrayPair[1]),
110138
assert.AssertionError
111139
);
140+
if (arrayPair[2]) {
141+
assert.partialDeepStrictEqual(arrayPair[1], arrayPair[0]);
142+
} else {
143+
assert.throws(
144+
makeBlock(assert.partialDeepStrictEqual, arrayPair[1], arrayPair[0]),
145+
assert.AssertionError
146+
);
147+
}
112148
});
113149
}
114150
});

0 commit comments

Comments
 (0)