-
Notifications
You must be signed in to change notification settings - Fork 608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
assertEquals
is very slow for large Set
/Map
obj due to O(n^2) comparison algorithm
#5903
Comments
PRs that improve the performance of |
Two possible directions to explore: const aEntries = [...a.entries()].sort();
const bEntries = [...b.entries()].sort();
for (const [idx, [aKey, aValue]] of aEntries.entries()) {
const [bKey, bValue] = bEntries[idx]!;
if (!compare(aKey, bKey) || !compare(aValue, bValue)) {
return false;
}
}
return true; This passes for all current tests but I think it'll fail for more complex examples as const keySet = new Set<unknown>();
const valSet = new Set<unknown>();
for (const input of [a, b]) {
for (const [key, value] of input.entries()) {
keySet.add(key);
valSet.add(value);
}
}
return keySet.size === a.size && valSet.size === a.size; This fails some of the current tests as sets are only unique by reference, not value. Basically, the problem in both cases is not having a unique-by-value, reference-equal, serialized form. |
I made this comparison with Lodash isEqual(). The difference is not as big I expected. Lodash code looks hacky. import "https://raw.githubusercontent.com/lodash/lodash/4.17.10-npm/lodash.js";
import { assertEquals } from "jsr:@std/assert";
const arr1 = Array.from({ length: 10_000 }, (_, i) => i)
const arr2 = [...arr1]
const set1 = new Set(arr1)
const set2 = new Set(arr2)
let result: boolean
console.time('compare set std')
assertEquals(set1, set2);
console.timeEnd('compare set std')
console.time('compare set lodash')
result = _.isEqual(set1, set2)
console.timeEnd('compare set lodash')
console.log(result)
|
A partial fix would be to have a fast path for cases where all keys of both are primitives, which would certainly have worked for the use case where I discovered this limitation, and maybe for most use cases where you'd be likely to want to compare large |
Talking only about Sets, if all the values in either Set of the comparison has only primitives (you don't even need to check both Sets, just one of them), then you can compare them instantly with: set1.symmetricDifference(set2).size == 0 I think this optimization is worth it because a Set usually has only primitives or only objects. I believe it's not common that a Set is populated with both primitives and objects. But I could be wrong. |
Yeah I misspoke, only one set/map needs checking for exclusively primitive keys (because if the other has some non-primitive keys, at least one of the primitive keys must be missing due to having already passed same-size check). Probably worth adding |
Describe the bug
assertEquals
is very slow when comparing largeSet
andMap
objects due to using anO(n^2)
comparison algorithm:std/assert/equal.ts
Lines 87 to 99 in 7e26160
It looks like there's the same issue in the
expect
version too.Steps to Reproduce
Results:
Results get exponentially worse as the size increases.
Expected behavior
assertEquals
ofSet
and array (orMap
and obj) to take approximately equal time.Environment
The text was updated successfully, but these errors were encountered: