From 841279d79c1b03fef4b864ee6681716220fa93b2 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 9 Feb 2023 22:07:11 +0100
Subject: [PATCH 1/4] benchmark: rework assert benchmarks for correctness

This reworks most assert benchmarks to provide more reliable test
cases that also test more cases than before while keeping the
runtime low.

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>
---
 benchmark/assert/deepequal-buffer.js          | 42 +++++++++----
 benchmark/assert/deepequal-object.js          | 33 +++++-----
 .../deepequal-prims-and-objs-big-loop.js      | 60 ++++++++++++++++---
 ...t.js => deepequal-simple-array-and-set.js} | 42 +++++--------
 benchmark/assert/deepequal-typedarrays.js     | 18 +++---
 benchmark/assert/ok.js                        | 17 ------
 benchmark/assert/throws.js                    | 42 -------------
 7 files changed, 123 insertions(+), 131 deletions(-)
 rename benchmark/assert/{deepequal-prims-and-objs-big-array-set.js => deepequal-simple-array-and-set.js} (54%)
 delete mode 100644 benchmark/assert/ok.js
 delete mode 100644 benchmark/assert/throws.js

diff --git a/benchmark/assert/deepequal-buffer.js b/benchmark/assert/deepequal-buffer.js
index 69cca91cc6d752..1b6aa3bee1d6ac 100644
--- a/benchmark/assert/deepequal-buffer.js
+++ b/benchmark/assert/deepequal-buffer.js
@@ -6,27 +6,47 @@ const bench = common.createBenchmark(main, {
   n: [2e4],
   len: [1e2, 1e3],
   strict: [0, 1],
-  method: ['deepEqual', 'notDeepEqual'],
+  arrayBuffer: [0, 1],
+  method: ['deepEqual', 'notDeepEqual', 'unequal_length'],
+}, {
+  combinationFilter: (p) => {
+    return p.strict === 1 || p.method === 'deepEqual';
+  },
 });
 
-function main({ len, n, method, strict }) {
-  const data = Buffer.allocUnsafe(len + 1);
-  const actual = Buffer.alloc(len);
-  const expected = Buffer.alloc(len);
-  const expectedWrong = Buffer.alloc(len + 1);
-  data.copy(actual);
-  data.copy(expected);
-  data.copy(expectedWrong);
+function main({ len, n, method, strict, arrayBuffer }) {
+  let actual = Buffer.alloc(len);
+  let expected = Buffer.alloc(len + Number(method === 'unequal_length'));
+
+
+  if (method === 'unequal_length') {
+    method = 'notDeepEqual';
+  }
+
+  for (let i = 0; i < len; i++) {
+    actual.writeInt8(i % 128, i);
+    expected.writeInt8(i % 128, i);
+  }
+
+  if (method.includes('not')) {
+    const position = Math.floor(len / 2);
+    expected[position] = expected[position] + 1;
+  }
 
   if (strict) {
     method = method.replace('eep', 'eepStrict');
   }
+
   const fn = assert[method];
-  const value2 = method.includes('not') ? expectedWrong : expected;
+
+  if (arrayBuffer) {
+    actual = actual.buffer;
+    expected = expected.buffer;
+  }
 
   bench.start();
   for (let i = 0; i < n; ++i) {
-    fn(actual, value2);
+    fn(actual, expected);
   }
   bench.end(n);
 }
diff --git a/benchmark/assert/deepequal-object.js b/benchmark/assert/deepequal-object.js
index 7418e2a745ba40..80bb50f7982529 100644
--- a/benchmark/assert/deepequal-object.js
+++ b/benchmark/assert/deepequal-object.js
@@ -4,14 +4,20 @@ const common = require('../common.js');
 const assert = require('assert');
 
 const bench = common.createBenchmark(main, {
-  n: [5e3],
-  size: [1e2, 1e3, 5e4],
-  strict: [0, 1],
+  n: [25, 2e2, 2e3],
+  size: [1e2, 1e3, 1e4],
+  strict: [1],
   method: ['deepEqual', 'notDeepEqual'],
+}, {
+  combinationFilter: (p) => {
+    return p.size === 1e4 && p.n === 25 ||
+           p.size === 1e3 && p.n === 2e2 ||
+           p.size === 1e2 && p.n === 2e3;
+  },
 });
 
-function createObj(source, add = '') {
-  return source.map((n) => ({
+function createObj(size, add = '') {
+  return Array.from({ length: size }, (n) => ({
     foo: 'yarp',
     nope: {
       bar: `123${add}`,
@@ -24,22 +30,17 @@ function createObj(source, add = '') {
 }
 
 function main({ size, n, method, strict }) {
-  const len = Math.min(Math.ceil(n / size), 20);
-
-  const source = Array.apply(null, Array(size));
-  const actual = createObj(source);
-  const expected = createObj(source);
-  const expectedWrong = createObj(source, '4');
-
   if (strict) {
     method = method.replace('eep', 'eepStrict');
   }
   const fn = assert[method];
-  const value2 = method.includes('not') ? expectedWrong : expected;
+
+  const actual = createObj(size);
+  const expected = method.includes('not') ? createObj(size, '4') : createObj(size);
 
   bench.start();
-  for (let i = 0; i < len; ++i) {
-    fn(actual, value2);
+  for (let i = 0; i < n; ++i) {
+    fn(actual, expected);
   }
-  bench.end(len);
+  bench.end(n);
 }
diff --git a/benchmark/assert/deepequal-prims-and-objs-big-loop.js b/benchmark/assert/deepequal-prims-and-objs-big-loop.js
index 2d01431b1fc563..1ab4ff4dd81f33 100644
--- a/benchmark/assert/deepequal-prims-and-objs-big-loop.js
+++ b/benchmark/assert/deepequal-prims-and-objs-big-loop.js
@@ -2,35 +2,77 @@
 const common = require('../common.js');
 const assert = require('assert');
 
+const circular = {};
+circular.circular = circular;
+const circular2 = {};
+circular2.circular = circular2;
+const notCircular = {};
+notCircular.circular = {};
+
 const primValues = {
-  'string': 'a',
-  'number': 1,
-  'object': { 0: 'a' },
+  'string': 'abcdef',
+  'number': 1_000,
+  'boolean': true,
+  'object': { property: 'abcdef' },
+  'object_other_property': { property: 'abcdef' },
+  'array': [1, 2, 3],
+  'set_object': new Set([[1]]),
+  'set_simple': new Set([1, 2, 3]),
+  'circular': circular,
+  'empty_object': {},
+  'regexp': /abc/i,
+  'date': new Date(),
+};
+
+const primValues2 = {
+  'object': { property: 'abcdef' },
   'array': [1, 2, 3],
+  'set_object': new Set([[1]]),
+  'set_simple': new Set([1, 3, 2]),
+  'circular': circular2,
+  'empty_object': {},
+  'regexp': /abc/i,
+  'date': new Date(primValues.date),
+};
+
+const primValuesUnequal = {
+  'string': 'abcdez',
+  'number': 1_001,
+  'boolean': false,
+  'object': { property2: 'abcdef' },
+  'array': [1, 3, 2],
+  'set_object': new Set([[2]]),
+  'set_simple': new Set([1, 4, 2]),
+  'circular': notCircular,
+  'empty_object': [],
+  'regexp': /abc/g,
+  'date': new Date(primValues.date.getTime() + 1),
 };
 
 const bench = common.createBenchmark(main, {
   primitive: Object.keys(primValues),
-  n: [2e4],
+  n: [1e5],
   strict: [0, 1],
   method: ['deepEqual', 'notDeepEqual'],
+}, {
+  combinationFilter: (p) => {
+    return p.strict === 1 || p.method === 'deepEqual';
+  },
 });
 
 function main({ n, primitive, method, strict }) {
   const prim = primValues[primitive];
-  const actual = prim;
-  const expected = prim;
-  const expectedWrong = 'b';
+  const actual = primValues2[primitive] ?? prim;
+  const expected = method.includes('not') ? primValuesUnequal[primitive] : prim;
 
   if (strict) {
     method = method.replace('eep', 'eepStrict');
   }
   const fn = assert[method];
-  const value2 = method.includes('not') ? expectedWrong : expected;
 
   bench.start();
   for (let i = 0; i < n; ++i) {
-    fn([actual], [value2]);
+    fn(actual, expected);
   }
   bench.end(n);
 }
diff --git a/benchmark/assert/deepequal-prims-and-objs-big-array-set.js b/benchmark/assert/deepequal-simple-array-and-set.js
similarity index 54%
rename from benchmark/assert/deepequal-prims-and-objs-big-array-set.js
rename to benchmark/assert/deepequal-simple-array-and-set.js
index ad049ded02ce9d..a1f6820696d7b8 100644
--- a/benchmark/assert/deepequal-prims-and-objs-big-array-set.js
+++ b/benchmark/assert/deepequal-simple-array-and-set.js
@@ -4,18 +4,10 @@ const common = require('../common.js');
 const { deepEqual, deepStrictEqual, notDeepEqual, notDeepStrictEqual } =
   require('assert');
 
-const primValues = {
-  'string': 'a',
-  'number': 1,
-  'object': { 0: 'a' },
-  'array': [1, 2, 3],
-};
-
 const bench = common.createBenchmark(main, {
-  primitive: Object.keys(primValues),
-  n: [25],
-  len: [2e4],
-  strict: [0, 1],
+  n: [5e2],
+  len: [1e4],
+  strict: [1],
   method: [
     'deepEqual_Array',
     'notDeepEqual_Array',
@@ -32,38 +24,32 @@ function run(fn, n, actual, expected) {
   bench.end(n);
 }
 
-function main({ n, len, primitive, method, strict }) {
-  const prim = primValues[primitive];
+function main({ n, len, method, strict }) {
   const actual = [];
   const expected = [];
-  const expectedWrong = [];
 
-  for (let x = 0; x < len; x++) {
-    actual.push(prim);
-    expected.push(prim);
-    expectedWrong.push(prim);
+  for (let i = 0; i < len; i++) {
+    actual.push(i);
+    expected.push(i);
+  }
+  if (method.includes('not')) {
+    expected[len - 1] += 1;
   }
-  expectedWrong.pop();
-  expectedWrong.push('b');
-
-  // Note: primitives are only added once to a set
-  const actualSet = new Set(actual);
-  const expectedSet = new Set(expected);
-  const expectedWrongSet = new Set(expectedWrong);
 
   switch (method) {
     case 'deepEqual_Array':
       run(strict ? deepStrictEqual : deepEqual, n, actual, expected);
       break;
     case 'notDeepEqual_Array':
-      run(strict ? notDeepStrictEqual : notDeepEqual, n, actual, expectedWrong);
+      run(strict ? notDeepStrictEqual : notDeepEqual, n, actual, expected);
       break;
     case 'deepEqual_Set':
-      run(strict ? deepStrictEqual : deepEqual, n, actualSet, expectedSet);
+      run(strict ? deepStrictEqual : deepEqual,
+          n, new Set(actual), new Set(expected));
       break;
     case 'notDeepEqual_Set':
       run(strict ? notDeepStrictEqual : notDeepEqual,
-          n, actualSet, expectedWrongSet);
+          n, new Set(actual), new Set(expected));
       break;
     default:
       throw new Error(`Unsupported method "${method}"`);
diff --git a/benchmark/assert/deepequal-typedarrays.js b/benchmark/assert/deepequal-typedarrays.js
index 188cfce695ed61..c06ff8fb2f3099 100644
--- a/benchmark/assert/deepequal-typedarrays.js
+++ b/benchmark/assert/deepequal-typedarrays.js
@@ -7,8 +7,7 @@ const bench = common.createBenchmark(main, {
     'Int8Array',
     'Uint8Array',
     'Float32Array',
-    'Float64Array',
-    'Uint8ClampedArray',
+    'Uint32Array',
   ],
   n: [5e2],
   strict: [0, 1],
@@ -23,21 +22,24 @@ function main({ type, n, len, method, strict }) {
   const clazz = global[type];
   const actual = new clazz(len);
   const expected = new clazz(len);
-  const expectedWrong = new clazz(len);
-  const wrongIndex = Math.floor(len / 2);
-  expectedWrong[wrongIndex] = 123;
 
   if (strict) {
     method = method.replace('eep', 'eepStrict');
   }
   const fn = assert[method];
-  const value2 = method.includes('not') ? expectedWrong : expected;
+
+  if (method.includes('not')) {
+    expected[Math.floor(len / 2)] = 123;
+  }
 
   bench.start();
   for (let i = 0; i < n; ++i) {
     actual[0] = i;
-    value2[0] = i;
-    fn(actual, value2);
+    expected[0] = i;
+    const pos = Math.ceil(len / 2) + 1;
+    actual[pos] = i;
+    expected[pos] = i;
+    fn(actual, expected);
   }
   bench.end(n);
 }
diff --git a/benchmark/assert/ok.js b/benchmark/assert/ok.js
deleted file mode 100644
index 42fd2e89b78d1e..00000000000000
--- a/benchmark/assert/ok.js
+++ /dev/null
@@ -1,17 +0,0 @@
-'use strict';
-
-const common = require('../common.js');
-const assert = require('assert');
-
-const bench = common.createBenchmark(main, { n: [1e5] });
-
-function main({ n }) {
-  bench.start();
-  for (let i = 0; i < n; ++i) {
-    if (i % 2 === 0)
-      assert(true);
-    else
-      assert(true, 'foo bar baz');
-  }
-  bench.end(n);
-}
diff --git a/benchmark/assert/throws.js b/benchmark/assert/throws.js
deleted file mode 100644
index 978ad2f1b8bef0..00000000000000
--- a/benchmark/assert/throws.js
+++ /dev/null
@@ -1,42 +0,0 @@
-'use strict';
-
-const common = require('../common.js');
-const { throws, doesNotThrow } = require('assert');
-
-const bench = common.createBenchmark(main, {
-  n: [1e4],
-  method: [ 'doesNotThrow', 'throws_TypeError', 'throws_RegExp' ],
-});
-
-function main({ n, method }) {
-  const throwError = () => { throw new TypeError('foobar'); };
-  const doNotThrowError = () => { return 'foobar'; };
-  const regExp = /foobar/;
-  const message = 'failure';
-
-  switch (method) {
-    case 'doesNotThrow':
-      bench.start();
-      for (let i = 0; i < n; ++i) {
-        doesNotThrow(doNotThrowError);
-      }
-      bench.end(n);
-      break;
-    case 'throws_TypeError':
-      bench.start();
-      for (let i = 0; i < n; ++i) {
-        throws(throwError, TypeError, message);
-      }
-      bench.end(n);
-      break;
-    case 'throws_RegExp':
-      bench.start();
-      for (let i = 0; i < n; ++i) {
-        throws(throwError, regExp, message);
-      }
-      bench.end(n);
-      break;
-    default:
-      throw new Error(`Unsupported method ${method}`);
-  }
-}

From a02b434966cd9e4ba1d4ed0d5d70c88c887991cd Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Fri, 10 Feb 2023 03:37:37 +0100
Subject: [PATCH 2/4] test: remove obsolete util.isDeepStrictEqual tests

These tests are a copy of the assert tests that verify the
deep equal comparison algorithm. There is no need to duplicate the
tests as changing one would require to change the tests in two
places without knowing which ones to actually rely upon.

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>
---
 test/parallel/test-util-isDeepStrictEqual.js | 419 -------------------
 1 file changed, 419 deletions(-)

diff --git a/test/parallel/test-util-isDeepStrictEqual.js b/test/parallel/test-util-isDeepStrictEqual.js
index da266fc84ce0bb..48b3116061932f 100644
--- a/test/parallel/test-util-isDeepStrictEqual.js
+++ b/test/parallel/test-util-isDeepStrictEqual.js
@@ -7,82 +7,6 @@ require('../common');
 const assert = require('assert');
 const util = require('util');
 
-class MyDate extends Date {
-  constructor(...args) {
-    super(...args);
-    this[0] = '1';
-  }
-}
-
-class MyRegExp extends RegExp {
-  constructor(...args) {
-    super(...args);
-    this[0] = '1';
-  }
-}
-
-{
-  const arr = new Uint8Array([120, 121, 122, 10]);
-  const buf = Buffer.from(arr);
-  // They have different [[Prototype]]
-  assert.strictEqual(util.isDeepStrictEqual(arr, buf), false);
-
-  const buf2 = Buffer.from(arr);
-  buf2.prop = 1;
-
-  assert.strictEqual(util.isDeepStrictEqual(buf2, buf), false);
-
-  const arr2 = new Uint8Array([120, 121, 122, 10]);
-  arr2.prop = 5;
-  assert.strictEqual(util.isDeepStrictEqual(arr, arr2), false);
-}
-
-{
-  const date = new Date('2016');
-
-  const date2 = new MyDate('2016');
-
-  // deepStrictEqual checks own properties
-  assert.strictEqual(util.isDeepStrictEqual(date, date2), false);
-  assert.strictEqual(util.isDeepStrictEqual(date2, date), false);
-}
-
-{
-  const re1 = new RegExp('test');
-  const re2 = new MyRegExp('test');
-
-  // deepStrictEqual checks all properties
-  assert.strictEqual(util.isDeepStrictEqual(re1, re2), false);
-}
-
-{
-  // For these cases, deepStrictEqual should throw.
-  const similar = new Set([
-    { 0: '1' },  // Object
-    { 0: 1 },  // Object
-    new String('1'),  // Object
-    ['1'],  // Array
-    [1],  // Array
-    new MyDate('2016'), // Date with this[0] = '1'
-    new MyRegExp('test'),  // RegExp with this[0] = '1'
-    new Int8Array([1]), // Int8Array
-    new Uint8Array([1]), // Uint8Array
-    new Int16Array([1]), // Int16Array
-    new Uint16Array([1]), // Uint16Array
-    new Int32Array([1]), // Int32Array
-    new Uint32Array([1]), // Uint32Array
-    Buffer.from([1]), // Buffer
-  ]);
-
-  for (const a of similar) {
-    for (const b of similar) {
-      if (a !== b) {
-        assert.strictEqual(util.isDeepStrictEqual(a, b), false);
-      }
-    }
-  }
-}
-
 function utilIsDeepStrict(a, b) {
   assert.strictEqual(util.isDeepStrictEqual(a, b), true);
   assert.strictEqual(util.isDeepStrictEqual(b, a), true);
@@ -93,345 +17,6 @@ function notUtilIsDeepStrict(a, b) {
   assert.strictEqual(util.isDeepStrictEqual(b, a), false);
 }
 
-// es6 Maps and Sets
-utilIsDeepStrict(new Set(), new Set());
-utilIsDeepStrict(new Map(), new Map());
-
-utilIsDeepStrict(new Set([1, 2, 3]), new Set([1, 2, 3]));
-notUtilIsDeepStrict(new Set([1, 2, 3]), new Set([1, 2, 3, 4]));
-notUtilIsDeepStrict(new Set([1, 2, 3, 4]), new Set([1, 2, 3]));
-utilIsDeepStrict(new Set(['1', '2', '3']), new Set(['1', '2', '3']));
-utilIsDeepStrict(new Set([[1, 2], [3, 4]]), new Set([[3, 4], [1, 2]]));
-
-{
-  const a = [ 1, 2 ];
-  const b = [ 3, 4 ];
-  const c = [ 1, 2 ];
-  const d = [ 3, 4 ];
-
-  utilIsDeepStrict(
-    { a: a, b: b, s: new Set([a, b]) },
-    { a: c, b: d, s: new Set([d, c]) }
-  );
-}
-
-utilIsDeepStrict(new Map([[1, 1], [2, 2]]), new Map([[1, 1], [2, 2]]));
-utilIsDeepStrict(new Map([[1, 1], [2, 2]]), new Map([[2, 2], [1, 1]]));
-notUtilIsDeepStrict(new Map([[1, 1], [2, 2]]), new Map([[1, 2], [2, 1]]));
-notUtilIsDeepStrict(
-  new Map([[[1], 1], [{}, 2]]),
-  new Map([[[1], 2], [{}, 1]])
-);
-
-notUtilIsDeepStrict(new Set([1]), [1]);
-notUtilIsDeepStrict(new Set(), []);
-notUtilIsDeepStrict(new Set(), {});
-
-notUtilIsDeepStrict(new Map([['a', 1]]), { a: 1 });
-notUtilIsDeepStrict(new Map(), []);
-notUtilIsDeepStrict(new Map(), {});
-
-notUtilIsDeepStrict(new Set(['1']), new Set([1]));
-
-notUtilIsDeepStrict(new Map([['1', 'a']]), new Map([[1, 'a']]));
-notUtilIsDeepStrict(new Map([['a', '1']]), new Map([['a', 1]]));
-notUtilIsDeepStrict(new Map([['a', '1']]), new Map([['a', 2]]));
-
-utilIsDeepStrict(new Set([{}]), new Set([{}]));
-
-// Ref: https://github.com/nodejs/node/issues/13347
-notUtilIsDeepStrict(
-  new Set([{ a: 1 }, { a: 1 }]),
-  new Set([{ a: 1 }, { a: 2 }])
-);
-notUtilIsDeepStrict(
-  new Set([{ a: 1 }, { a: 1 }, { a: 2 }]),
-  new Set([{ a: 1 }, { a: 2 }, { a: 2 }])
-);
-notUtilIsDeepStrict(
-  new Map([[{ x: 1 }, 5], [{ x: 1 }, 5]]),
-  new Map([[{ x: 1 }, 5], [{ x: 2 }, 5]])
-);
-
-notUtilIsDeepStrict(new Set([3, '3']), new Set([3, 4]));
-notUtilIsDeepStrict(new Map([[3, 0], ['3', 0]]), new Map([[3, 0], [4, 0]]));
-
-notUtilIsDeepStrict(
-  new Set([{ a: 1 }, { a: 1 }, { a: 2 }]),
-  new Set([{ a: 1 }, { a: 2 }, { a: 2 }])
-);
-
-// Mixed primitive and object keys
-utilIsDeepStrict(
-  new Map([[1, 'a'], [{}, 'a']]),
-  new Map([[1, 'a'], [{}, 'a']])
-);
-utilIsDeepStrict(
-  new Set([1, 'a', [{}, 'a']]),
-  new Set([1, 'a', [{}, 'a']])
-);
-
-// This is an awful case, where a map contains multiple equivalent keys:
-notUtilIsDeepStrict(
-  new Map([[1, 'a'], ['1', 'b']]),
-  new Map([['1', 'a'], [true, 'b']])
-);
-notUtilIsDeepStrict(
-  new Set(['a']),
-  new Set(['b'])
-);
-utilIsDeepStrict(
-  new Map([[{}, 'a'], [{}, 'b']]),
-  new Map([[{}, 'b'], [{}, 'a']])
-);
-notUtilIsDeepStrict(
-  new Map([[true, 'a'], ['1', 'b'], [1, 'a']]),
-  new Map([['1', 'a'], [1, 'b'], [true, 'a']])
-);
-notUtilIsDeepStrict(
-  new Map([[true, 'a'], ['1', 'b'], [1, 'c']]),
-  new Map([['1', 'a'], [1, 'b'], [true, 'a']])
-);
-
-// Similar object keys
-notUtilIsDeepStrict(
-  new Set([{}, {}]),
-  new Set([{}, 1])
-);
-notUtilIsDeepStrict(
-  new Set([[{}, 1], [{}, 1]]),
-  new Set([[{}, 1], [1, 1]])
-);
-notUtilIsDeepStrict(
-  new Map([[{}, 1], [{}, 1]]),
-  new Map([[{}, 1], [1, 1]])
-);
-notUtilIsDeepStrict(
-  new Map([[{}, 1], [true, 1]]),
-  new Map([[{}, 1], [1, 1]])
-);
-
-// Similar primitive key / values
-notUtilIsDeepStrict(
-  new Set([1, true, false]),
-  new Set(['1', 0, '0'])
-);
-notUtilIsDeepStrict(
-  new Map([[1, 5], [true, 5], [false, 5]]),
-  new Map([['1', 5], [0, 5], ['0', 5]])
-);
-
-// Undefined value in Map
-utilIsDeepStrict(
-  new Map([[1, undefined]]),
-  new Map([[1, undefined]])
-);
-notUtilIsDeepStrict(
-  new Map([[1, null]]),
-  new Map([['1', undefined]])
-);
-notUtilIsDeepStrict(
-  new Map([[1, undefined]]),
-  new Map([[2, undefined]])
-);
-
-// null as key
-utilIsDeepStrict(
-  new Map([[null, 3]]),
-  new Map([[null, 3]])
-);
-notUtilIsDeepStrict(
-  new Map([[null, undefined]]),
-  new Map([[undefined, null]])
-);
-notUtilIsDeepStrict(
-  new Set([null]),
-  new Set([undefined])
-);
-
-// GH-6416. Make sure circular refs don't throw.
-{
-  const b = {};
-  b.b = b;
-  const c = {};
-  c.b = c;
-
-  utilIsDeepStrict(b, c);
-
-  const d = {};
-  d.a = 1;
-  d.b = d;
-  const e = {};
-  e.a = 1;
-  e.b = {};
-
-  notUtilIsDeepStrict(d, e);
-}
-
-// GH-14441. Circular structures should be consistent
-{
-  const a = {};
-  const b = {};
-  a.a = a;
-  b.a = {};
-  b.a.a = a;
-  utilIsDeepStrict(a, b);
-}
-
-{
-  const a = new Set();
-  const b = new Set();
-  const c = new Set();
-  a.add(a);
-  b.add(b);
-  c.add(a);
-  utilIsDeepStrict(b, c);
-}
-
-// GH-7178. Ensure reflexivity of deepEqual with `arguments` objects.
-{
-  const args = (function() { return arguments; })();
-  notUtilIsDeepStrict([], args);
-}
-
-// More checking that arguments objects are handled correctly
-{
-  // eslint-disable-next-line func-style
-  const returnArguments = function() { return arguments; };
-
-  const someArgs = returnArguments('a');
-  const sameArgs = returnArguments('a');
-  const diffArgs = returnArguments('b');
-
-  notUtilIsDeepStrict(someArgs, ['a']);
-  notUtilIsDeepStrict(someArgs, { '0': 'a' });
-  notUtilIsDeepStrict(someArgs, diffArgs);
-  utilIsDeepStrict(someArgs, sameArgs);
-}
-
-{
-  const values = [
-    123,
-    Infinity,
-    0,
-    null,
-    undefined,
-    false,
-    true,
-    {},
-    [],
-    () => {},
-  ];
-  utilIsDeepStrict(new Set(values), new Set(values));
-  utilIsDeepStrict(new Set(values), new Set(values.reverse()));
-
-  const mapValues = values.map((v) => [v, { a: 5 }]);
-  utilIsDeepStrict(new Map(mapValues), new Map(mapValues));
-  utilIsDeepStrict(new Map(mapValues), new Map(mapValues.reverse()));
-}
-
-{
-  const s1 = new Set();
-  const s2 = new Set();
-  s1.add(1);
-  s1.add(2);
-  s2.add(2);
-  s2.add(1);
-  utilIsDeepStrict(s1, s2);
-}
-
-{
-  const m1 = new Map();
-  const m2 = new Map();
-  const obj = { a: 5, b: 6 };
-  m1.set(1, obj);
-  m1.set(2, 'hi');
-  m1.set(3, [1, 2, 3]);
-
-  m2.set(2, 'hi'); // different order
-  m2.set(1, obj);
-  m2.set(3, [1, 2, 3]); // Deep equal, but not reference equal.
-
-  utilIsDeepStrict(m1, m2);
-}
-
-{
-  const m1 = new Map();
-  const m2 = new Map();
-
-  // m1 contains itself.
-  m1.set(1, m1);
-  m2.set(1, new Map());
-
-  notUtilIsDeepStrict(m1, m2);
-}
-
-{
-  const map1 = new Map([[1, 1]]);
-  const map2 = new Map([[1, '1']]);
-  assert.strictEqual(util.isDeepStrictEqual(map1, map2), false);
-}
-
-{
-  // Two equivalent sets / maps with different key/values applied shouldn't be
-  // the same. This is a terrible idea to do in practice, but deepEqual should
-  // still check for it.
-  const s1 = new Set();
-  const s2 = new Set();
-  s1.x = 5;
-  notUtilIsDeepStrict(s1, s2);
-
-  const m1 = new Map();
-  const m2 = new Map();
-  m1.x = 5;
-  notUtilIsDeepStrict(m1, m2);
-}
-
-{
-  // Circular references.
-  const s1 = new Set();
-  s1.add(s1);
-  const s2 = new Set();
-  s2.add(s2);
-  utilIsDeepStrict(s1, s2);
-
-  const m1 = new Map();
-  m1.set(2, m1);
-  const m2 = new Map();
-  m2.set(2, m2);
-  utilIsDeepStrict(m1, m2);
-
-  const m3 = new Map();
-  m3.set(m3, 2);
-  const m4 = new Map();
-  m4.set(m4, 2);
-  utilIsDeepStrict(m3, m4);
-}
-
-// Handle sparse arrays
-/* eslint-disable no-sparse-arrays */
-utilIsDeepStrict([1, , , 3], [1, , , 3]);
-notUtilIsDeepStrict([1, , , 3], [1, , , 3, , , ]);
-/* eslint-enable no-sparse-arrays */
-
-// Handle different error messages
-{
-  const err1 = new Error('foo1');
-  const err2 = new Error('foo2');
-  const err3 = new TypeError('foo1');
-  notUtilIsDeepStrict(err1, err2, assert.AssertionError);
-  notUtilIsDeepStrict(err1, err3, assert.AssertionError);
-  notUtilIsDeepStrict(err1, {}, assert.AssertionError);
-}
-
-// Handle NaN
-assert.strictEqual(util.isDeepStrictEqual(NaN, NaN), true);
-assert.strictEqual(util.isDeepStrictEqual({ a: NaN }, { a: NaN }), true);
-assert.strictEqual(
-  util.isDeepStrictEqual([ 1, 2, NaN, 4 ], [ 1, 2, NaN, 4 ]),
-  true
-);
-
 // Handle boxed primitives
 {
   const boxedString = new String('test');
@@ -478,10 +63,6 @@ assert.strictEqual(
   notUtilIsDeepStrict(symbolish, new String('fhqwhgads'));
 }
 
-// Minus zero
-notUtilIsDeepStrict(0, -0);
-utilIsDeepStrict(-0, -0);
-
 // Handle symbols (enumerable only)
 {
   const symbol1 = Symbol();

From 449e9f448937ed1843e4edd61b19590177925ea1 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 9 Feb 2023 22:12:25 +0100
Subject: [PATCH 3/4] 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>
---
 benchmark/assert/deepequal-object.js |   3 +-
 doc/api/assert.md                    |  12 +-
 lib/internal/util/comparisons.js     | 270 +++++++++++++++------------
 test/parallel/test-assert-deep.js    |  22 ++-
 4 files changed, 175 insertions(+), 132 deletions(-)

diff --git a/benchmark/assert/deepequal-object.js b/benchmark/assert/deepequal-object.js
index 80bb50f7982529..4238129b292b45 100644
--- a/benchmark/assert/deepequal-object.js
+++ b/benchmark/assert/deepequal-object.js
@@ -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;
   },
 });
 
diff --git a/doc/api/assert.md b/doc/api/assert.md
index d40300cc3fdb1b..34107c7730fdd9 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -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.
diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js
index aa0973baa45c8b..96b5210c8be096 100644
--- a/lib/internal/util/comparisons.js
+++ b/lib/internal/util/comparisons.js
@@ -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 val2 !== 'object' || val1 === null || val2 === null) {
-      return false;
-    }
-    if (ObjectGetPrototypeOf(val1) !== ObjectGetPrototypeOf(val2)) {
+    if (typeof val1 === 'number') {
+      return NumberIsNaN(val1) && NumberIsNaN(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) {
+  const isArrayLikeObject = aKeys !== undefined;
+
+  if (aKeys === undefined) {
     aKeys = ObjectKeys(val1);
-    const bKeys = ObjectKeys(val2);
+  }
 
-    // The pair must have the same number of owned properties.
-    if (aKeys.length !== bKeys.length) {
-      return false;
+  // Cheap key test
+  if (aKeys.length > 0) {
+    for (const key of aKeys) {
+      if (!ObjectPrototypePropertyIsEnumerable(val2, key)) {
+        return false;
+      }
     }
   }
 
-  // Cheap key test
-  let i = 0;
-  for (; i < aKeys.length; i++) {
-    if (!ObjectPrototypePropertyIsEnumerable(val2, aKeys[i])) {
+  if (!isArrayLikeObject) {
+    // The pair must have the same number of owned properties.
+    if (aKeys.length !== ObjectKeys(val2).length) {
       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 (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;
+    return objEquiv(val1, val2, strict, aKeys, memos, iterationType);
+  }
+
+  if (memos.set === undefined) {
+    if (memos.deep === false) {
+      if (memos.a === val1) {
+        return memos.b === val2;
       }
-    }
-    memos.position++;
+      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);
   }
 
-  memos.val1.set(val1, memos.position);
-  memos.val2.set(val2, memos.position);
+  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;
-
-  if (iterationType === kIsSet) {
-    if (!setEquiv(a, b, strict, memos)) {
-      return false;
-    }
-  } else if (iterationType === kIsMap) {
-    if (!mapEquiv(a, b, strict, memos)) {
-      return false;
+  // 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;
+      }
     }
-  } 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;
 }
 
diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js
index 013c7985f51d2a..d4334b584549b9 100644
--- a/test/parallel/test-assert-deep.js
+++ b/test/parallel/test-assert-deep.js
@@ -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

From 575784b4cff6eb7fbb16e41cdd2b576563756a09 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 14 Feb 2023 19:49:23 +0100
Subject: [PATCH 4/4] assert,util: revert recursive breaking change

This commit is there to be reverted after merging. It makes it easy
to backport the overall PR and allows easy forward fixing.

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>
---
 doc/api/assert.md                 | 12 ++----------
 lib/internal/util/comparisons.js  | 12 +++---------
 test/parallel/test-assert-deep.js |  6 +++---
 3 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/doc/api/assert.md b/doc/api/assert.md
index 34107c7730fdd9..d40300cc3fdb1b 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -546,10 +546,6 @@ 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.
@@ -621,7 +617,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 either side encounters a circular
+* Recursion stops when both sides differ or both sides encounter a circular
   reference.
 * Implementation does not test the [`[[Prototype]]`][prototype-spec] of
   objects.
@@ -731,10 +727,6 @@ 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.
@@ -788,7 +780,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 either side encounters a circular
+* Recursion stops when both sides differ or both sides encounter a circular
   reference.
 * [`WeakMap`][] and [`WeakSet`][] comparison does not rely on their values. See
   below for further details.
diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js
index 96b5210c8be096..65dd54589008f3 100644
--- a/lib/internal/util/comparisons.js
+++ b/lib/internal/util/comparisons.js
@@ -343,10 +343,7 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
   if (memos.set === undefined) {
     if (memos.deep === false) {
       if (memos.a === val1) {
-        return memos.b === val2;
-      }
-      if (memos.b === val2) {
-        return false;
+        if (memos.b === val2) return true;
       }
       memos.c = val1;
       memos.d = val2;
@@ -366,12 +363,9 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
 
   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;
+  if (originalSize === set.size) {
+    return true;
   }
 
   const areEq = objEquiv(val1, val2, strict, aKeys, memos, iterationType);
diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js
index d4334b584549b9..f7489993aa60ba 100644
--- a/test/parallel/test-assert-deep.js
+++ b/test/parallel/test-assert-deep.js
@@ -425,7 +425,7 @@ assertNotDeepOrStrict(
   b.a = {};
   b.a.a = a;
 
-  assertNotDeepOrStrict(a, b);
+  assertDeepAndStrictEqual(a, b);
 }
 
 {
@@ -435,7 +435,7 @@ assertNotDeepOrStrict(
   b.a = b;
   const c = {};
   c.a = a;
-  assertNotDeepOrStrict(b, c);
+  assertDeepAndStrictEqual(b, c);
 }
 
 {
@@ -445,7 +445,7 @@ assertNotDeepOrStrict(
   b.add(b);
   const c = new Set();
   c.add(a);
-  assertNotDeepOrStrict(b, c);
+  assertDeepAndStrictEqual(b, c);
 }
 
 // https://github.com/nodejs/node-v0.x-archive/pull/7178