assert: refactor to reduce unecessary code paths

The lazy loading is not needed as the errors themself lazy
load assert. Therefore the circle is working as intended
even without this lazy loading.

Improve Array, object, ArrayBuffer, Set and Map performance
in all deepEqual checks by removing unecessary code paths and
by moving expensive checks further back.

Improve throws and doesNotThrow performance by removing dead
code and simplifying the overall logic.

PR-URL: https://github.com/nodejs/node/pull/13973
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

1 files changed, 177 insertions, 188 deletions

diff --git a/lib/assert.js b/lib/assert.js
index 902bb82ce2..b66061b29b 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -25,13 +25,7 @@ const util = require('util');
 const { isSet, isMap } = process.binding('util');
 const { objectToString } = require('internal/util');
 const { Buffer } = require('buffer');
-
-var errors;
-function lazyErrors() {
-  if (!errors)
-    errors = require('internal/errors');
-  return errors;
-}
+const errors = require('internal/errors');
 
 // The assert module provides functions that throw
 // AssertionError's when particular conditions are not met. The
@@ -51,7 +45,6 @@ const assert = module.exports = ok;
 // display purposes.
 
 function innerFail(actual, expected, message, operator, stackStartFunction) {
-  const errors = lazyErrors();
   throw new errors.AssertionError({
     message,
     actual,
@@ -82,7 +75,7 @@ assert.fail = fail;
 // new assert.AssertionError({ message: message,
 //                             actual: actual,
 //                             expected: expected });
-assert.AssertionError = lazyErrors().AssertionError;
+assert.AssertionError = errors.AssertionError;
 
 
 // Pure assertion tests whether a value is truthy, as determined
@@ -122,23 +115,35 @@ assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {
   }
 };
 
+// Check if they have the same source and flags
 function areSimilarRegExps(a, b) {
   return a.source === b.source && a.flags === b.flags;
 }
 
+// For small buffers it's faster to compare the buffer in a loop.
+// The c++ barrier takes the advantage of the faster compare otherwise.
+// 300 was the number after which compare became faster.
 function areSimilarTypedArrays(a, b) {
+  const len = a.byteLength;
+  if (len !== b.byteLength) {
+    return false;
+  }
+  if (len < 300) {
+    for (var offset = 0; offset < len; offset++) {
+      if (a[offset] !== b[offset]) {
+        return false;
+      }
+    }
+    return true;
+  }
   return compare(Buffer.from(a.buffer,
                              a.byteOffset,
-                             a.byteLength),
+                             len),
                  Buffer.from(b.buffer,
                              b.byteOffset,
                              b.byteLength)) === 0;
 }
 
-function isNullOrNonObj(object) {
-  return object === null || typeof object !== 'object';
-}
-
 function isFloatTypedArrayTag(tag) {
   return tag === '[object Float32Array]' || tag === '[object Float64Array]';
 }
@@ -147,109 +152,125 @@ function isArguments(tag) {
   return tag === '[object Arguments]';
 }
 
-function innerDeepEqual(actual, expected, strict, memos) {
-  // All identical values are equivalent, as determined by ===.
-  if (actual === expected) {
-    return true;
-  }
-
-  // For primitives / functions
-  // (determined by typeof value !== 'object'),
-  // or null, equivalence is determined by === or ==.
-  if (isNullOrNonObj(actual) && isNullOrNonObj(expected)) {
-    // eslint-disable-next-line eqeqeq
-    return strict ? actual === expected : actual == expected;
-  }
+function isObjectOrArrayTag(tag) {
+  return tag === '[object Array]' || tag === '[object Object]';
+}
 
-  // If they bypass the previous check, then at least
-  // one of them must be an non-null object.
-  // If the other one is null or undefined, they must not be equal.
-  if (actual === null || actual === undefined ||
-      expected === null || expected === undefined)
+// Notes: Type tags are historical [[Class]] properties that can be set by
+// FunctionTemplate::SetClassName() in C++ or Symbol.toStringTag in JS
+// and retrieved using Object.prototype.toString.call(obj) in JS
+// See https://tc39.github.io/ecma262/#sec-object.prototype.tostring
+// for a list of tags pre-defined in the spec.
+// There are some unspecified tags in the wild too (e.g. typed array tags).
+// Since tags can be altered, they only serve fast failures
+//
+// Typed arrays and buffers are checked by comparing the content in their
+// underlying ArrayBuffer. This optimization requires that it's
+// reasonable to interpret their underlying memory in the same way,
+// which is checked by comparing their type tags.
+// (e.g. a Uint8Array and a Uint16Array with the same memory content
+// could still be different because they will be interpreted differently)
+// Never perform binary comparisons for Float*Arrays, though,
+// since e.g. +0 === -0 is true despite the two values' bit patterns
+// not being identical.
+//
+// For strict comparison, objects should have
+// a) The same built-in type tags
+// b) The same prototypes.
+function strictDeepEqual(actual, expected) {
+  if (actual === null || expected === null ||
+    typeof actual !== 'object' || typeof expected !== 'object') {
     return false;
-
-  // Notes: Type tags are historical [[Class]] properties that can be set by
-  // FunctionTemplate::SetClassName() in C++ or Symbol.toStringTag in JS
-  // and retrieved using Object.prototype.toString.call(obj) in JS
-  // See https://tc39.github.io/ecma262/#sec-object.prototype.tostring
-  // for a list of tags pre-defined in the spec.
-  // There are some unspecified tags in the wild too (e.g. typed array tags).
-  // Since tags can be altered, they only serve fast failures
+  }
   const actualTag = objectToString(actual);
   const expectedTag = objectToString(expected);
 
-  // Passing null or undefined to Object.getPrototypeOf() will throw
-  // so this must done after previous checks.
-  // For strict comparison, objects should have
-  // a) The same prototypes.
-  // b) The same built-in type tags
-  if (strict) {
-    if (Object.getPrototypeOf(actual) !== Object.getPrototypeOf(expected)) {
+  if (actualTag !== expectedTag) {
+    return false;
+  }
+  if (Object.getPrototypeOf(actual) !== Object.getPrototypeOf(expected)) {
+    return false;
+  }
+  if (isObjectOrArrayTag(actualTag)) {
+    // Skip testing the part below and continue in the callee function.
+    return;
+  }
+  if (util.isDate(actual)) {
+    if (actual.getTime() !== expected.getTime()) {
       return false;
     }
-
-    if (actualTag !== expectedTag) {
+  } else if (util.isRegExp(actual)) {
+    if (!areSimilarRegExps(actual, expected)) {
+      return false;
+    }
+  } else if (!isFloatTypedArrayTag(actualTag) && ArrayBuffer.isView(actual)) {
+    if (!areSimilarTypedArrays(actual, expected)) {
       return false;
     }
-  }
 
-  // Do fast checks for builtin types.
-  // If they don't match, they must not be equal.
-  // If they match, return true for non-strict comparison.
-  // For strict comparison we need to exam further.
+    // Buffer.compare returns true, so actual.length === expected.length
+    // if they both only contain numeric keys, we don't need to exam further
+    if (Object.keys(actual).length === actual.length &&
+        Object.keys(expected).length === expected.length) {
+      return true;
+    }
+  }
+}
 
-  // If both values are Date objects,
-  // check if the time underneath are equal first.
+function looseDeepEqual(actual, expected) {
+  if (actual === null || typeof actual !== 'object') {
+    if (expected === null || typeof expected !== 'object') {
+      // eslint-disable-next-line eqeqeq
+      return actual == expected;
+    }
+    return false;
+  }
+  if (expected === null || typeof expected !== 'object') {
+    return false;
+  }
   if (util.isDate(actual) && util.isDate(expected)) {
     if (actual.getTime() !== expected.getTime()) {
       return false;
-    } else if (!strict) {
-      return true;  // Skip further checks for non-strict comparison.
     }
+    return true;
   }
-
-  // If both values are RegExp, check if they have
-  // the same source and flags first
   if (util.isRegExp(actual) && util.isRegExp(expected)) {
     if (!areSimilarRegExps(actual, expected)) {
       return false;
-    } else if (!strict) {
-      return true;  // Skip further checks for non-strict comparison.
     }
+    return true;
   }
-
+  const actualTag = objectToString(actual);
+  const expectedTag = objectToString(expected);
+  if (actualTag === expectedTag) {
+    if (!isFloatTypedArrayTag(actualTag) && !isObjectOrArrayTag(actualTag) &&
+      ArrayBuffer.isView(actual)) {
+      return areSimilarTypedArrays(actual, expected);
+    }
   // Ensure reflexivity of deepEqual with `arguments` objects.
   // See https://github.com/nodejs/node-v0.x-archive/pull/7178
-  if (isArguments(actualTag) !== isArguments(expectedTag)) {
+  } else if (isArguments(actualTag) || isArguments(expectedTag)) {
     return false;
   }
+}
 
-  // Check typed arrays and buffers by comparing the content in their
-  // underlying ArrayBuffer. This optimization requires that it's
-  // reasonable to interpret their underlying memory in the same way,
-  // which is checked by comparing their type tags.
-  // (e.g. a Uint8Array and a Uint16Array with the same memory content
-  // could still be different because they will be interpreted differently)
-  // Never perform binary comparisons for Float*Arrays, though,
-  // since e.g. +0 === -0 is true despite the two values' bit patterns
-  // not being identical.
-  if (ArrayBuffer.isView(actual) && ArrayBuffer.isView(expected) &&
-      actualTag === expectedTag && !isFloatTypedArrayTag(actualTag)) {
-    if (!areSimilarTypedArrays(actual, expected)) {
-      return false;
-    } else if (!strict) {
-      return true;  // Skip further checks for non-strict comparison.
-    }
+function innerDeepEqual(actual, expected, strict, memos) {
+  // All identical values are equivalent, as determined by ===.
+  if (actual === expected) {
+    return true;
+  }
 
-    // Buffer.compare returns true, so actual.length === expected.length
-    // if they both only contain numeric keys, we don't need to exam further
-    if (Object.keys(actual).length === actual.length &&
-        Object.keys(expected).length === expected.length) {
-      return true;
-    }
+  // Returns a boolean if (not) equal and undefined in case we have to check
+  // further.
+  const partialCheck = strict ?
+    strictDeepEqual(actual, expected) :
+    looseDeepEqual(actual, expected);
+
+  if (partialCheck !== undefined) {
+    return partialCheck;
   }
 
-  // For all other Object pairs, including Array objects and Maps,
+  // For all remaining Object pairs, including Array, objects and Maps,
   // equivalence is determined by having:
   // a) The same number of owned enumerable properties
   // b) The same set of keys/indexes (although not necessarily the same order)
@@ -258,24 +279,42 @@ function innerDeepEqual(actual, expected, strict, memos) {
   // Note: this accounts for both named and indexed properties on Arrays.
 
   // Use memos to handle cycles.
-  if (!memos) {
+  if (memos === undefined) {
     memos = {
       actual: new Map(),
       expected: new Map(),
       position: 0
     };
   } else {
+    if (memos.actual.has(actual)) {
+      return memos.actual.get(actual) === memos.expected.get(expected);
+    }
     memos.position++;
   }
 
-  if (memos.actual.has(actual)) {
-    return memos.actual.get(actual) === memos.expected.get(expected);
+  const aKeys = Object.keys(actual);
+  const bKeys = Object.keys(expected);
+  var i;
+
+  // The pair must have the same number of owned properties
+  // (keys incorporates hasOwnProperty).
+  if (aKeys.length !== bKeys.length)
+    return false;
+
+  // Cheap key test:
+  const keys = {};
+  for (i = 0; i < aKeys.length; i++) {
+    keys[aKeys[i]] = true;
+  }
+  for (i = 0; i < aKeys.length; i++) {
+    if (keys[bKeys[i]] === undefined)
+      return false;
   }
 
   memos.actual.set(actual, memos.position);
   memos.expected.set(expected, memos.position);
 
-  const areEq = objEquiv(actual, expected, strict, memos);
+  const areEq = objEquiv(actual, expected, strict, aKeys, memos);
 
   memos.actual.delete(actual);
   memos.expected.delete(expected);
@@ -285,24 +324,20 @@ function innerDeepEqual(actual, expected, strict, memos) {
 
 function setHasSimilarElement(set, val1, usedEntries, strict, memo) {
   if (set.has(val1)) {
-    if (usedEntries)
+    if (usedEntries !== null)
       usedEntries.add(val1);
     return true;
   }
 
   // In strict mode the only things which can match a primitive or a function
   // will already be detected by set.has(val1).
-  if (strict && (util.isPrimitive(val1) || util.isFunction(val1)))
+  if (strict && (typeof val1 !== 'object' || val1 === null))
     return false;
 
   // Otherwise go looking.
   for (const val2 of set) {
-    if (usedEntries && usedEntries.has(val2))
-      continue;
-
-    if (innerDeepEqual(val1, val2, strict, memo)) {
-      if (usedEntries)
-        usedEntries.add(val2);
+    if (!usedEntries.has(val2) && innerDeepEqual(val1, val2, strict, memo)) {
+      usedEntries.add(val2);
       return true;
     }
   }
@@ -329,10 +364,10 @@ function setEquiv(a, b, strict, memo) {
   // that case this initialization is done lazily to avoid the allocation &
   // bookkeeping cost. Unfortunately, we can't get away with that in non-strict
   // mode.
-  let usedEntries = null;
+  let usedEntries = strict === true ? null : new Set();
 
   for (const val1 of a) {
-    if (usedEntries == null && (!strict || typeof val1 === 'object'))
+    if (usedEntries === null && typeof val1 === 'object')
       usedEntries = new Set();
 
     // If the value doesn't exist in the second set by reference, and its an
@@ -358,26 +393,22 @@ function mapHasSimilarEntry(map, key1, item1, usedEntries, strict, memo) {
   // doing it here improves performance of the common case when reference-equal
   // keys exist (which includes all primitive-valued keys).
   if (map.has(key1) && innerDeepEqual(item1, map.get(key1), strict, memo)) {
-    if (usedEntries)
+    if (usedEntries !== null)
       usedEntries.add(key1);
     return true;
   }
 
-  if (strict && (util.isPrimitive(key1) || util.isFunction(key1)))
+  if (strict && (typeof key1 !== 'object' || key1 === null))
     return false;
 
   for (const [key2, item2] of map) {
-    // This case is checked above.
-    if (key2 === key1)
-      continue;
-
-    if (usedEntries && usedEntries.has(key2))
+    // The first part is checked above.
+    if (key2 === key1 || usedEntries.has(key2))
       continue;
 
     if (innerDeepEqual(key1, key2, strict, memo) &&
         innerDeepEqual(item1, item2, strict, memo)) {
-      if (usedEntries)
-        usedEntries.add(key2);
+      usedEntries.add(key2);
       return true;
     }
   }
@@ -393,56 +424,31 @@ function mapEquiv(a, b, strict, memo) {
   if (a.size !== b.size)
     return false;
 
-  let usedEntries = null;
+  let usedEntries = strict === true ? null : new Set();
 
-  for (const [key1, item1] of a) {
-    if (usedEntries == null && (!strict || typeof key1 === 'object'))
+  for (const [key, item] of a) {
+    if (usedEntries === null && typeof key === 'object')
       usedEntries = new Set();
 
     // Just like setEquiv above, this hunt makes this function O(n^2) when
     // using objects and lists as keys
-    if (!mapHasSimilarEntry(b, key1, item1, usedEntries, strict, memo))
+    if (!mapHasSimilarEntry(b, key, item, usedEntries, strict, memo))
       return false;
   }
 
   return true;
 }
 
-function objEquiv(a, b, strict, actualVisitedObjects) {
-  // If one of them is a primitive, the other must be the same.
-  if (util.isPrimitive(a) || util.isPrimitive(b))
-    return a === b;
-
-  const aKeys = Object.keys(a);
-  const bKeys = Object.keys(b);
-  var key, i;
-
-  // The pair must have the same number of owned properties
-  // (keys incorporates hasOwnProperty).
-  if (aKeys.length !== bKeys.length)
-    return false;
-
-  // The pair must have the same set of keys (although not
-  // necessarily in the same order).
-  aKeys.sort();
-  bKeys.sort();
-  // Cheap key test:
-  for (i = aKeys.length - 1; i >= 0; i--) {
-    if (aKeys[i] !== bKeys[i])
-      return false;
-  }
-
+function objEquiv(a, b, strict, keys, memos) {
   // Sets and maps don't have their entries accessible via normal object
   // properties.
   if (isSet(a)) {
-    if (!isSet(b) || !setEquiv(a, b, strict, actualVisitedObjects))
+    if (!isSet(b) || !setEquiv(a, b, strict, memos))
       return false;
   } else if (isSet(b)) {
     return false;
-  }
-
-  if (isMap(a)) {
-    if (!isMap(b) || !mapEquiv(a, b, strict, actualVisitedObjects))
+  } else if (isMap(a)) {
+    if (!isMap(b) || !mapEquiv(a, b, strict, memos))
       return false;
   } else if (isMap(b)) {
     return false;
@@ -450,9 +456,9 @@ function objEquiv(a, b, strict, actualVisitedObjects) {
 
   // The pair must have equivalent values for every corresponding key.
   // Possibly expensive deep test:
-  for (i = aKeys.length - 1; i >= 0; i--) {
-    key = aKeys[i];
-    if (!innerDeepEqual(a[key], b[key], strict, actualVisitedObjects))
+  for (var i = 0; i < keys.length; i++) {
+    const key = keys[i];
+    if (!innerDeepEqual(a[key], b[key], strict, memos))
       return false;
   }
   return true;
@@ -489,45 +495,32 @@ assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
 };
 
 function expectedException(actual, expected) {
-  // actual is guaranteed to be an Error object, but we need to check expected.
-  if (!expected) {
-    return false;
-  }
-
-  if (objectToString(expected) === '[object RegExp]') {
+  if (typeof expected !== 'function') {
+    // Should be a RegExp, if not fail hard
     return expected.test(actual);
   }
-
-  try {
-    if (actual instanceof expected) {
-      return true;
-    }
-  } catch (e) {
-    // Ignore. The instanceof check doesn't work for arrow functions.
+  // Guard instanceof against arrow functions as they don't have a prototype.
+  if (expected.prototype !== undefined && actual instanceof expected) {
+    return true;
   }
-
   if (Error.isPrototypeOf(expected)) {
     return false;
   }
-
   return expected.call({}, actual) === true;
 }
 
 function tryBlock(block) {
-  var error;
   try {
     block();
   } catch (e) {
-    error = e;
+    return e;
   }
-  return error;
 }
 
 function innerThrows(shouldThrow, block, expected, message) {
-  var actual;
+  var details = '';
 
   if (typeof block !== 'function') {
-    const errors = lazyErrors();
     throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'block', 'function',
                                block);
   }
@@ -537,28 +530,24 @@ function innerThrows(shouldThrow, block, expected, message) {
     expected = null;
   }
 
-  actual = tryBlock(block);
-
-  message = (expected && expected.name ? ' (' + expected.name + ')' : '') +
-            (message ? ': ' + message : '.');
-
-  if (shouldThrow && !actual) {
-    innerFail(actual, expected, 'Missing expected exception' + message, fail);
-  }
-
-  const userProvidedMessage = typeof message === 'string';
-  const isUnwantedException = !shouldThrow && util.isError(actual);
-  const isUnexpectedException = !shouldThrow && actual && !expected;
-
-  if ((isUnwantedException &&
-      userProvidedMessage &&
-      expectedException(actual, expected)) ||
-      isUnexpectedException) {
-    innerFail(actual, expected, 'Got unwanted exception' + message, fail);
-  }
+  const actual = tryBlock(block);
 
-  if ((shouldThrow && actual && expected &&
-      !expectedException(actual, expected)) || (!shouldThrow && actual)) {
+  if (shouldThrow === true) {
+    if (actual === undefined) {
+      if (expected && expected.name) {
+        details += ` (${expected.name})`;
+      }
+      details += message ? `: ${message}` : '.';
+      fail(actual, expected, `Missing expected exception${details}`, fail);
+    }
+    if (expected && expectedException(actual, expected) === false) {
+      throw actual;
+    }
+  } else if (actual !== undefined) {
+    if (!expected || expectedException(actual, expected)) {
+      details = message ? `: ${message}` : '.';
+      fail(actual, expected, `Got unwanted exception${details}`, fail);
+    }
     throw actual;
   }
 }