Skip to content

Commit 5203bb0

Browse files
BridgeARrefack
authored andcommitted
assert: improve deepEqual Set and Map worst case
This change improves the algorithm for the worst case from O(n^2) to O(n log n) by using a lazily initiated set with object or not strict equal primitives keys. In addition a few comments got fixed and a statement simplified. PR-URL: #14258 Reviewed-By: Refael Ackermann <[email protected]>
1 parent 8ddb725 commit 5203bb0

File tree

2 files changed

+254
-86
lines changed

2 files changed

+254
-86
lines changed

lib/assert.js

Lines changed: 170 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ const errors = require('internal/errors');
3333

3434
const assert = module.exports = ok;
3535

36-
// At present only the three keys mentioned above are used and
37-
// understood by the spec. Implementations or sub modules can pass
38-
// other keys to the AssertionError's constructor - they will be
39-
// ignored.
40-
4136
// All of the following functions must throw an AssertionError
4237
// when a corresponding condition is not met, with a message that
4338
// may be undefined if not provided. All assertion methods provide
@@ -120,9 +115,9 @@ function areSimilarRegExps(a, b) {
120115
return a.source === b.source && a.flags === b.flags;
121116
}
122117

123-
// For small buffers it's faster to compare the buffer in a loop.
124-
// The c++ barrier takes the advantage of the faster compare otherwise.
125-
// 300 was the number after which compare became faster.
118+
// For small buffers it's faster to compare the buffer in a loop. The c++
119+
// barrier including the Buffer.from operation takes the advantage of the faster
120+
// compare otherwise. 300 was the number after which compare became faster.
126121
function areSimilarTypedArrays(a, b) {
127122
const len = a.byteLength;
128123
if (len !== b.byteLength) {
@@ -229,21 +224,15 @@ function looseDeepEqual(actual, expected) {
229224
return false;
230225
}
231226
if (util.isDate(actual) && util.isDate(expected)) {
232-
if (actual.getTime() !== expected.getTime()) {
233-
return false;
234-
}
235-
return true;
227+
return actual.getTime() === expected.getTime();
236228
}
237229
if (util.isRegExp(actual) && util.isRegExp(expected)) {
238-
if (!areSimilarRegExps(actual, expected)) {
239-
return false;
240-
}
241-
return true;
230+
return areSimilarRegExps(actual, expected);
242231
}
243232
const actualTag = objectToString(actual);
244233
const expectedTag = objectToString(expected);
245234
if (actualTag === expectedTag) {
246-
if (!isFloatTypedArrayTag(actualTag) && !isObjectOrArrayTag(actualTag) &&
235+
if (!isObjectOrArrayTag(actualTag) && !isFloatTypedArrayTag(actualTag) &&
247236
ArrayBuffer.isView(actual)) {
248237
return areSimilarTypedArrays(actual, expected);
249238
}
@@ -322,29 +311,38 @@ function innerDeepEqual(actual, expected, strict, memos) {
322311
return areEq;
323312
}
324313

325-
function setHasSimilarElement(set, val1, usedEntries, strict, memo) {
326-
if (set.has(val1)) {
327-
if (usedEntries !== null)
328-
usedEntries.add(val1);
329-
return true;
330-
}
331-
332-
// In strict mode the only things which can match a primitive or a function
333-
// will already be detected by set.has(val1).
334-
if (strict && (typeof val1 !== 'object' || val1 === null))
335-
return false;
336-
337-
// Otherwise go looking.
314+
function setHasEqualElement(set, val1, strict, memo) {
315+
// Go looking.
338316
for (const val2 of set) {
339-
if (!usedEntries.has(val2) && innerDeepEqual(val1, val2, strict, memo)) {
340-
usedEntries.add(val2);
317+
if (innerDeepEqual(val1, val2, strict, memo)) {
318+
// Remove the matching element to make sure we do not check that again.
319+
set.delete(val2);
341320
return true;
342321
}
343322
}
344323

345324
return false;
346325
}
347326

327+
// Note: we actually run this multiple times for each loose key!
328+
// This is done to prevent slowing down the average case.
329+
function setHasLoosePrim(a, b, val) {
330+
const altValues = findLooseMatchingPrimitives(val);
331+
if (altValues === undefined)
332+
return false;
333+
334+
var matches = 1;
335+
for (var i = 0; i < altValues.length; i++) {
336+
if (b.has(altValues[i])) {
337+
matches--;
338+
}
339+
if (a.has(altValues[i])) {
340+
matches++;
341+
}
342+
}
343+
return matches === 0;
344+
}
345+
348346
function setEquiv(a, b, strict, memo) {
349347
// This code currently returns false for this pair of sets:
350348
// assert.deepEqual(new Set(['1', 1]), new Set([1]))
@@ -356,59 +354,124 @@ function setEquiv(a, b, strict, memo) {
356354
if (a.size !== b.size)
357355
return false;
358356

359-
// This is a set of the entries in b which have been consumed in our pairwise
360-
// comparison.
361-
//
357+
// This is a lazily initiated Set of entries which have to be compared
358+
// pairwise.
359+
var set = null;
362360
// When the sets contain only value types (eg, lots of numbers), and we're in
363-
// strict mode, we don't need to match off the entries in a pairwise way. In
364-
// that case this initialization is done lazily to avoid the allocation &
365-
// bookkeeping cost. Unfortunately, we can't get away with that in non-strict
366-
// mode.
367-
let usedEntries = strict === true ? null : new Set();
368-
369-
for (const val1 of a) {
370-
if (usedEntries === null && typeof val1 === 'object')
371-
usedEntries = new Set();
372-
373-
// If the value doesn't exist in the second set by reference, and its an
374-
// object or an array we'll need to go hunting for something thats
375-
// deep-equal to it. Note that this is O(n^2) complexity, and will get
376-
// slower if large, very similar sets / maps are nested inside.
377-
// Unfortunately there's no real way around this.
378-
if (!setHasSimilarElement(b, val1, usedEntries, strict, memo))
361+
// strict mode or if all entries strictly match, we don't need to match the
362+
// entries in a pairwise way. In that case this initialization is done lazily
363+
// to avoid the allocation & bookkeeping cost.
364+
for (const val of a) {
365+
// Note: Checking for the objects first improves the performance for object
366+
// heavy sets but it is a minor slow down for primitives. As they are fast
367+
// to check this improves the worst case scenario instead.
368+
if (typeof val === 'object' && val !== null) {
369+
if (set === null) {
370+
set = new Set();
371+
}
372+
// If the specified value doesn't exist in the second set its an not null
373+
// object (or non strict only: a not matching primitive) we'll need to go
374+
// hunting for something thats deep-(strict-)equal to it. To make this
375+
// O(n log n) complexity we have to copy these values in a new set first.
376+
set.add(val);
377+
} else if (!b.has(val) && (strict || !setHasLoosePrim(a, b, val))) {
379378
return false;
379+
}
380+
}
381+
382+
if (set !== null) {
383+
for (const val of b) {
384+
// In non-strict-mode we have to check if a primitive value is already
385+
// matching and only if it's not, go hunting for it.
386+
if (typeof val === 'object' && val !== null) {
387+
if (!setHasEqualElement(set, val, strict, memo))
388+
return false;
389+
} else if (!a.has(val) && (strict || !setHasLoosePrim(b, a, val))) {
390+
return false;
391+
}
392+
}
380393
}
381394

382395
return true;
383396
}
384397

385-
function mapHasSimilarEntry(map, key1, item1, usedEntries, strict, memo) {
386-
// To be able to handle cases like:
387-
// Map([[1, 'a'], ['1', 'b']]) vs Map([['1', 'a'], [1, 'b']])
388-
// or:
389-
// Map([[{}, 'a'], [{}, 'b']]) vs Map([[{}, 'b'], [{}, 'a']])
390-
// ... we need to consider *all* matching keys, not just the first we find.
398+
function findLooseMatchingPrimitives(prim) {
399+
var values, number;
400+
switch (typeof prim) {
401+
case 'number':
402+
values = ['' + prim];
403+
if (prim === 1 || prim === 0)
404+
values.push(Boolean(prim));
405+
return values;
406+
case 'string':
407+
number = +prim;
408+
if ('' + number === prim) {
409+
values = [number];
410+
if (number === 1 || number === 0)
411+
values.push(Boolean(number));
412+
}
413+
return values;
414+
case 'undefined':
415+
return [null];
416+
case 'object': // Only pass in null as object!
417+
return [undefined];
418+
case 'boolean':
419+
number = +prim;
420+
return [number, '' + number];
421+
}
422+
}
391423

392-
// This check is not strictly necessary. The loop performs this check, but
393-
// doing it here improves performance of the common case when reference-equal
394-
// keys exist (which includes all primitive-valued keys).
395-
if (map.has(key1) && innerDeepEqual(item1, map.get(key1), strict, memo)) {
396-
if (usedEntries !== null)
397-
usedEntries.add(key1);
398-
return true;
424+
// This is a ugly but relatively fast way to determine if a loose equal entry
425+
// actually has a correspondent matching entry. Otherwise checking for such
426+
// values would be way more expensive (O(n^2)).
427+
// Note: we actually run this multiple times for each loose key!
428+
// This is done to prevent slowing down the average case.
429+
function mapHasLoosePrim(a, b, key1, memo, item1, item2) {
430+
const altKeys = findLooseMatchingPrimitives(key1);
431+
if (altKeys === undefined)
432+
return false;
433+
434+
const setA = new Set();
435+
const setB = new Set();
436+
437+
var keyCount = 1;
438+
439+
setA.add(item1);
440+
if (b.has(key1)) {
441+
keyCount--;
442+
setB.add(item2);
399443
}
400444

401-
if (strict && (typeof key1 !== 'object' || key1 === null))
445+
for (var i = 0; i < altKeys.length; i++) {
446+
const key2 = altKeys[i];
447+
if (a.has(key2)) {
448+
keyCount++;
449+
setA.add(a.get(key2));
450+
}
451+
if (b.has(key2)) {
452+
keyCount--;
453+
setB.add(b.get(key2));
454+
}
455+
}
456+
if (keyCount !== 0 || setA.size !== setB.size)
402457
return false;
403458

404-
for (const [key2, item2] of map) {
405-
// The first part is checked above.
406-
if (key2 === key1 || usedEntries.has(key2))
407-
continue;
459+
for (const val of setA) {
460+
if (!setHasEqualElement(setB, val, false, memo))
461+
return false;
462+
}
463+
464+
return true;
465+
}
408466

467+
function mapHasEqualEntry(set, map, key1, item1, strict, memo) {
468+
// To be able to handle cases like:
469+
// Map([[{}, 'a'], [{}, 'b']]) vs Map([[{}, 'b'], [{}, 'a']])
470+
// ... we need to consider *all* matching keys, not just the first we find.
471+
for (const key2 of set) {
409472
if (innerDeepEqual(key1, key2, strict, memo) &&
410-
innerDeepEqual(item1, item2, strict, memo)) {
411-
usedEntries.add(key2);
473+
innerDeepEqual(item1, map.get(key2), strict, memo)) {
474+
set.delete(key2);
412475
return true;
413476
}
414477
}
@@ -419,21 +482,45 @@ function mapHasSimilarEntry(map, key1, item1, usedEntries, strict, memo) {
419482
function mapEquiv(a, b, strict, memo) {
420483
// Caveat: In non-strict mode, this implementation does not handle cases
421484
// where maps contain two equivalent-but-not-reference-equal keys.
422-
//
423-
// For example, maps like this are currently considered not equivalent:
424485
if (a.size !== b.size)
425486
return false;
426487

427-
let usedEntries = strict === true ? null : new Set();
428-
429-
for (const [key, item] of a) {
430-
if (usedEntries === null && typeof key === 'object')
431-
usedEntries = new Set();
432-
433-
// Just like setEquiv above, this hunt makes this function O(n^2) when
434-
// using objects and lists as keys
435-
if (!mapHasSimilarEntry(b, key, item, usedEntries, strict, memo))
488+
var set = null;
489+
490+
for (const [key, item1] of a) {
491+
// By directly retrieving the value we prevent another b.has(key) check in
492+
// almost all possible cases.
493+
const item2 = b.get(key);
494+
if (item2 === undefined) {
495+
// Just like setEquiv above but in addition we have to make sure the
496+
// values are also equal.
497+
if (typeof key === 'object' && key !== null) {
498+
if (set === null) {
499+
set = new Set();
500+
}
501+
set.add(key);
502+
// Note: we do not have to pass memo in this case as at least one item
503+
// is undefined.
504+
} else if ((!innerDeepEqual(item1, item2, strict) || !b.has(key)) &&
505+
(strict || !mapHasLoosePrim(a, b, key, memo, item1))) {
506+
return false;
507+
}
508+
} else if (!innerDeepEqual(item1, item2, strict, memo) &&
509+
(strict || !mapHasLoosePrim(a, b, key, memo, item1, item2))) {
436510
return false;
511+
}
512+
}
513+
514+
if (set !== null) {
515+
for (const [key, item] of b) {
516+
if (typeof key === 'object' && key !== null) {
517+
if (!mapHasEqualEntry(set, a, key, item, strict, memo))
518+
return false;
519+
} else if (!a.has(key) &&
520+
(strict || !mapHasLoosePrim(b, a, key, memo, item))) {
521+
return false;
522+
}
523+
}
437524
}
438525

439526
return true;
@@ -445,12 +532,10 @@ function objEquiv(a, b, strict, keys, memos) {
445532
if (isSet(a)) {
446533
if (!isSet(b) || !setEquiv(a, b, strict, memos))
447534
return false;
448-
} else if (isSet(b)) {
449-
return false;
450535
} else if (isMap(a)) {
451536
if (!isMap(b) || !mapEquiv(a, b, strict, memos))
452537
return false;
453-
} else if (isMap(b)) {
538+
} else if (isSet(b) || isMap(b)) {
454539
return false;
455540
}
456541

0 commit comments

Comments
 (0)