assert,util: improve deep comparison performance

PR-URL: #61076
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>

Author: BridgeAR

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);
 }
 
@@ -126,6 +125,11 @@ const {
   getOwnNonIndexProperties,
 } = internalBinding('util');
 
+let kKeyObject;
+let kExtractable;
+let kAlgorithm;
+let kKeyUsages;
+
 const kStrict = 2;
 const kStrictWithoutPrototypes = 3;
 const kLoose = 0;
@@ -151,7 +155,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;
@@ -186,11 +190,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) {
@@ -224,7 +224,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);
@@ -394,8 +394,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) ||
@@ -411,18 +413,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);
       }
     }
@@ -454,32 +449,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;
+      }
     }
   }
 
@@ -641,16 +623,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) {
@@ -792,18 +772,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) {
@@ -892,17 +870,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;
       }
     }
@@ -973,8 +955,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;
       }
     }
@@ -1023,10 +1008,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);

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
+        );
+      }
     });
   }
 });