Repository navigation
Conversation
Bun__deepEquals called specialObjectsDequal a second time, with the operands swapped, when the first call answered std::nullopt. That call lets a special right operand reject a left operand of another type. For a pair of one JSType it ran the same arm again. On node's entry points the Set and Map arms answer std::nullopt after they compared every entry, so each level of nesting doubled the work. - The dispatch makes the swapped call only when the two JSTypes differ. - The comparisons that covered one direction now cover both: boxed Symbol and BigInt, URLSearchParams and Headers. - node's Set and Map arms walk the right collection and look each entry up in the left one, as node does. An entry without an identical counterpart matches only an entry that the other side lacks. When two or more entries needed such a match, the walk also runs in the other direction. - A Map key that holds undefined is settled by has(), without a scan. - On node's entry points the Error arm leaves the own enumerable properties to the generic walk. - The copy of the wrapper block in specialObjectsDequalSlow was unreachable and is removed.
Collaborator
Author
|
Reproduced with this script. It prints four results of import util from "node:util";
const answer = compare => {
try {
return String(compare());
} catch (error) {
return error.name;
}
};
// 1. Reads of one value under 1, 4 and 10 nested Maps.
let reads = 0;
const nested = depth => {
let value = { get leaf() { reads++; return 1; } };
for (let i = 0; i < depth; i++) value = new Map([["k", value]]);
return value;
};
const counts = [1, 4, 10].map(depth => {
reads = 0;
util.isDeepStrictEqual(nested(depth), nested(depth));
return reads;
});
console.log("1.", counts.join(" / "));
// 2. A Map that holds itself, against a copy.
const selfHolding = () => {
const map = new Map();
return map.set("self", map);
};
console.log("2.", answer(() => util.isDeepStrictEqual(selfHolding(), selfHolding())));
// 3. Two Maps with the same entries in another order. One key is the same object on both sides.
const key = { id: 1 };
console.log("3.", answer(() => util.isDeepStrictEqual(new Map([[key, "x"], [{ id: 1 }, "y"]]), new Map([[{ id: 1 }, "y"], [key, "x"]]))));
// 4. Sets that hold deep-equal members in different counts.
console.log("4.", answer(() => util.isDeepStrictEqual(new Set([{ a: 1 }, { a: 1 }, { a: 2 }]), new Set([{ a: 1 }, { a: 2 }, { a: 2 }]))));
// 5. Two Sets of 1,000 objects, 20 comparisons.
const set = () => new Set(Array.from({ length: 1000 }, (_, id) => ({ id })));
const a = set();
const b = set();
const start = performance.now();
for (let i = 0; i < 20; i++) util.isDeepStrictEqual(a, b);
console.log("5.", ((performance.now() - start) / 20).toFixed(2), "ms");
Rows 1 to 3 are regressions of #34660. Rows 4 and 5 are older than that. Column 5 is the median of 3 runs of the script, and a run includes the first call. #38510 is folded into this PR. This PR is a draft until the self-review of the folded diff is done. |
Collaborator
Author
|
Updated 4:05 PM PT - Oct 16th, 2026
❌ @robobun, your commit 6a0989c has 1 failures in
🧪 To try this PR locally: bunx bun-pr 45457That installs a local version of the PR into your bun-45457 --bun |
A failing expect() matcher prints a diff of both values. The negated matcher passes for an unequal pair, so the test no longer prints one.
…y points assert.deepStrictEqual and util.isDeepStrictEqual accepted Sets that hold deep-equal objects in different counts, and rejected Maps whose deep-equal keys hold different values. Each entry that the lookup missed scanned the other collection. The two arms now walk the right collection and settle each entry that the left one holds. The entries that are left pair one-to-one, in the probe order of node's setObjectEquiv and mapObjectEquiv: the first open candidate, then the last one, then the rest from the back. The entry points that follow Jest keep their code.
…n node's entry points
This was referenced Oct 24, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #28760, related to #34830
Problem
Bun__deepEquals(src/jsc/bindings/bindings.cpp) compares a Set or a Map once per argument order forassert.deepStrictEqualandutil.isDeepStrictEqual(node's entry points). 10 nested Maps read a leaf 2,048 times (node: 2). A Map that holds itself throwsRangeError: Maximum call stack size exceeded.Fix
Bun__deepEqualsswaps the operands only when their internal types (JSType) differ.nodeSetEquivandnodeMapEquivsettle the entries that both sides hold.NodeUnorderedPairingpairs the rest one-to-one. Those Sets take 63.9 µs.test/js/node/assert/deep-equal.test.ts,test/js/bun/bun-object/deep-equals.test.ts(main fails 113 tests),test/js/node/assert/,expect.test.js. Self-reviewed: 38 concerns raised, 37 addressed. Not done: sharing the rule withassert.partialDeepStrictEqual.Background
checkPrototypesmarks the twoBun__deepEqualsinstantiations behind node's entry points. The other five follow Jest.Bun.deepEquals(its answer would depend on insertion order).Downsides
Notes
Repro
Rows 1 to 3 are regressions of #34660. Rows 4 and 5 are older than that. Column 5 is the median of 3 runs of the script, and a run includes the first call. The bench below has the warm numbers. No user reported the slowdown. It was found by measurement.
What changes in
bindings.cppBun__deepEquals, all seven instantiations): afterspecialObjectsDequal(c1, c2)answersstd::nullopt, the swapped callspecialObjectsDequal(c2, c1)runs only when the two cells have different JSTypes. The swapped call exists so that a special right operand can reject a left operand of another type (fix:Bun.deepEqualson empty objects with the same prototype #16894). For two cells of one type it ran every arm that ends instd::nullopta second time.specialObjectsDequalSlowis gone.Rules of the Set and Map arms on node's entry points
nodeSetEquivandnodeMapEquiv, for node'ssetEquivandmapEquiv): walk the right collection and look each entry up in the left one. A Set member that the left Set holds is settled. A Map key that the left Map holds is settled when the two values are deep-equal. A collection that this loop settles completely never reaches the pairing.typeof) ends the comparison: nothing else on the left equals it. Every other such entry becomes a candidate. Its key goes into aMarkedArgumentBuffer.addTwinlooks for a left entry that can be its partner. Maps that differ in one value have none, and the comparison ends there.claimCandidates, for node'ssetObjectEquivandmapObjectEquiv): walk the left collection. Every object entry that the right side does not settle claims one candidate throughBun::PendingEntries::claimWith: front guess, back guess, then a scan from the back with a swap-remove. The value of a candidate is read from the right Map at each probe, as in node.extraChecks). So does this PR. A left entry that is not an object is not looked up at all: when the right side does not hold it, a candidate stays open at the end and the answer is not equal.break, and the generic walk compares the own properties once.static_assertkeeps a loose node mode and asymmetric matchers out of this pairing.Verdicts
Bun.deepEqualsloose, strict and strict withskipPrototype,expect().toEqual,expect().toStrictEqual): 0 of 57 rows of a sweep change in either argument order.assert.deepEqualuses the loose instantiation.nameagainst an own enumerablemessage) in the five positions, which the Error arm accepts on main too.constructoragainst{}, and forObject.create(null)against a Proxy whosegetPrototypeOfanswers null. Inside a Map or a Set, bun now gives node's answer in each order. On main it answered not equal in both.Measurements (release builds of main a407eae and of this PR at 297815a, which is that tree with the
bindings.cppandPendingEntries.hof this PR. The branch merged main d940fbe after that, and the merge does not touch the code of this PR.)size bun-profile): text 106,033,357 -> 106,039,477 B.nm -S -C: +7,354 B over 25 symbols. New:nodeMapEquiv3,776,nodeSetEquiv1,955,claimCandidates1,940,mapValueOrUndefined1,436,holds1,241,mapHolds1,172, aWTF::Vector<size_t, 8>growth function 447,entriesEqual363,addCandidate331, an outlinedURLSearchParams::get180.specialObjectsDequalSlowgrows from 10,945 to 12,134. The sevenBun__deepEqualsgrow by 6 to 129 each. The two node instantiations ofspecialObjectsDequalshrink from 4,525 to 933 each, and the five others by 4 each.bench/snippets/assert.mjs, median of 5 interleaved rounds. On the rows below 2 µs, two runs of one binary differ by up to 25% on this machine.deepStrictEqual, two-key objectdeepStrictEqual, 10 nested MapsdeepStrictEqual, Map of 10,000 entriesdeepStrictEqual, Map subclass of 10,000 entriesdeepStrictEqual, Set of 10,000 stringsdeepStrictEqual, Set of 1,000 objectsdeepStrictEqual, the same Set in a scattered orderdeepStrictEqual, Map of 1,000 object keysdeepStrictEqual, Set of 2,000 held and 8 other objectsdeepStrictEqual, 10 nested Sets of two objectsnotDeepStrictEqual, Sets of 16 objects, none equalnotDeepStrictEqual, Maps of 16 object keys, none equalnotDeepStrictEqual, changed copy of a Map of 1,000 object keysbun-profile, because perf and valgrind are not available on the machine. A malloc that takes its slow path adds about 90 to a count.{a: 1, b: {c: 2}}:Bun.deepEquals1,466 -> 1,386, strict 3,767 -> 3,673, strict withskipPrototype1,468 -> 1,386,expect().toEqual2,047 -> 1,965,expect().toStrictEqual4,356 -> 4,263,util.isDeepStrictEqual2,286 -> 2,162, withskipPrototype1,777 -> 1,664.Bun.deepEquals:[1, 2, 3]1,639 -> 1,594,Intl.Collator733 -> 617,Object(Symbol.iterator)779 -> 623,Bun.Glob977 -> 710. Throughutil.isDeepStrictEqual: Date 1,113 -> 981, RegExp 1,872 -> 1,625, boxed Number 1,104 -> 966, Error 7,306 -> 7,311,Uint8Arrayof 64 bytes 4,151 -> 4,149.{a: 1}against[1]411 -> 416, Proxy of{a: 1}against{a: 1}4,206 -> 4,211,{a: 1}againstnew Map()350 -> 354, the reverse 301 -> 302 (allBun.deepEquals). Throughutil.isDeepStrictEqualthe same four are 891 -> 890, 3,581 -> 3,582, 743 -> 742 and 743 -> 742.expect().toEqualon collections: Set of 96 strings 22,429 -> 22,429, Map of 96 strings 38,024 -> 38,024, Map of 96 strings to{v}81,698 -> 77,570, one-object Set 1,968 -> 1,925.Bun.deepEqualson 16-object Sets 103,380 -> 98,165.util.isDeepStrictEqual, settled by the first loop: Set of 96 strings 44,918 -> 21,826. Map of 96 strings 72,652 -> 35,810. Map of 96 strings to{v}233,728 -> 110,396. 96 object keys that both sides hold 41,596 -> 20,562. 4 and 8 nested string-keyed Maps 41,742 and 693,125 -> 5,810 and 10,518.addTwincosts about 160 to 185 instructions in the first two cases and one key comparison (about 1,200) in the third.util.isDeepStrictEqualequal node v26.3.0 in 105 of 105 rows: Set members, Map keys and Map values under deep-equal keys, n = 3, 4, 8, 16 and 64, in seven orders. For n = 64: same order 128, reversed 130, rotated by one 2,176 (main: 8,320 in every order).assert.notDeepStrictEqual, as in node), and twice in 7 shapes on the six Jest-rule spellings. Main reads 2,048 on node's spellings.toStringon a boxed String with an own property runs 2 times per comparison on all seven entry points (main: 4 on the five strict ones, bun 1.3.14: 2). Node does not call it.Differential against node v26.3.0 (
util.isDeepStrictEqual, 8 seeded configurations of 100,000 random pairs, both argument orders. Of the pairs, 30% are a copy of the left side in another order, 25% are such a copy in which one entry is a second copy of another entry, and 45% are built independently. The member pool has primitives, plain, dictionary-mode, undefined-property and non-enumerable-property objects, arrays, boxed values, nested Sets and Maps, and members shared by identity.nullandundefinedmembers are out, because node v26.3.0 throws a TypeError on some collections that hold them.)compareSmallSetsshortcut.skipPrototype: main 29,878 and 97,966, this PR 556 and 0.null,undefined, functions, symbols and-0in the pool.The script
The 8 configurations are
set 1 100000 5 0.25 all,set 2 100000 8 0.5 all,set 3 100000 9 0.3 dense,set 4 100000 10 0.5 dense,map 5 100000 5 0.25 all,map 6 100000 8 0.5 all,map 7 100000 9 0.3 denseandmap 8 100000 10 0.5 dense. Each runtime runs each configuration in one process.Answers that differ from node v26.3.0, each pinned by a test
new Set([shared, {a: 1}])against a Set of the same shape, wheresharedis{a: 1}: equal here. Node'scompareSmallSetscompares the shared member with the copy first and answers not equal.new Set([shared, undefined])againstnew Set([shared, {a: 1}]): not equal here. The same shortcut reads past the end of the other Set.nullorundefinedentry against one of objects only: not equal here in both argument orders. Node throws a TypeError in one order (assert.deepStrictEqual / util.isDeepStrictEqual throw TypeError on Maps with null keys nodejs/node#64433, fixed in v26.10.0).A getter that changes a collection during the comparison
The candidates are copies of the right keys, and the right values are read when an entry is compared. 23 test rows hold node's answer and bun's for a getter that changes the left or the right collection during the pairing. The two are the same in the 16 rows where node has an answer. In the other 7 rows node throws a TypeError: the left side gains an entry, and node reads past the end of its list. There bun answers whether the right side holds every left entry that has no partner yet. 24 more rows do the same for a getter that runs during the first loop: bun equals node in 22, and in 2 a Map gains an own property, which node compares before the entries. Two rows differ because node compares a one-entry Map and a Set of fewer than three members with shortcuts. One test clears a right Set of 40 members and runs a GC during the pairing, so that only the candidate buffer keeps the members alive.
Tests of the tests
38 single-clause changes of the code of this PR were built, and each one ran against the two test files: 5 changes of the dispatch test and the three repairs, 3 of the reverse walk of the Error arm, and 30 of the Set and Map rules. 35 fail a test (one of them on an assertion of the debug build). 2 change no answer: they remove an optimization (the lookups that
extraChecksskips, and the return when the last candidate is claimed). 1 makes the entries of a Set or a Map join the cycle stack, which this PR leaves as on main.Other PRs
toEqualincorrect comparison of maps with referential keys. #34830 is its issue). The two PRs do not depend on each other. The second to merge needs a rebase.PendingEntries.hhas the same content as the file in assert.partialDeepStrictEqual: settle Set members and Map keys by lookup instead of rescanning per entry #38554 at 235f97c. The PR that lands second drops its copy.toStringcall of the boxed String arm. Make loose deepEquals reject a non-enumerable match in either argument order #41540 and node errors: make code an own enumerable property on ERR_* errors and warnings #41737 change the Error arm and thecodeofERR_*errors. deepEquals: compare DOMException name, message and cause #39308 adds a DOMException arm. Draft bun test: Vitest compatibility #44833 changes the arm for global objects. A test row here (globalThisagainst a vm global with the same enumerable properties) fails if an arm tests only its left operand.test/parallel/test-assert-deep.jsof node v26.3.0 passes 30 of its 54 tests with and without this PR, and the same 24 fail. This PR adds no passing upstream test.Not in this PR
assert.deepEqualkeepsBun.deepEqualssemantics (docs/runtime/nodejs-compat.mdx). Its rows that differ from node arelooseBugrows in the test file.Bun.deepEqualsandexpect()still scan the other collection for each entry that the lookup misses, and a Map key that holdsundefinedstill takes that scan there. deepEquals: match a Map entry on its key and value together #44636 has the lookup fix. A pairing there is a separate decision.causeis compared by the Error arm and again by its property walk, so a chain of such Errors costs 2^depth on every entry point. This is older than node compat batch: callback-throw dispatch, Assert class + native deep-equality parity, Intl gate + URL/buffer fallout, compile cache, watch kill-signal, profilers (+98 tests) #34660 (bun 1.3.14 has it) and goes to the work on Make loose deepEquals reject a non-enumerable match in either argument order #41540.AggregateError.errorsis not compared on any entry point.ERR_*error that bun creates holdscodeon a prototype. So it does not equal an error with the same class, message and owncode, where node answers equal. node errors: make code an own enumerable property on ERR_* errors and warnings #41737 owns that.assert.partialDeepStrictEqual(NodeUtilTypesModule.cpp). assert.partialDeepStrictEqual: settle Set members and Map keys by lookup instead of rescanning per entry #38554 shares the header.Questions for a maintainer
WTF::Vector<JSValue>. The comment there says that the left collection roots them. A getter that removes entries from the left collection and then triggers a GC leaves the copies beyond the inline capacity of 8 without a root. Here the copies are in aMarkedArgumentBuffer.if constexpr (isStrict)branches are for a loose node mode, which thestatic_asserthere keeps out.Self-review
A review of the first version of this PR (the dispatch change with a second walk in the two node arms) raised 38 concerns. Its main finding: that interim rule accepted a four-entry Map pair that node and main reject, and it left Sets and Maps of two or more objects at 2^depth. The pairing of #38510 replaces it, and a test row pins that pair. The other concerns and what was done:
ERR_*error equal a bare error of its class and message. It is out. The reverse walk that replaces it keeps main's answer, and two rows pin it.deep-equals.test.ts, and it fails on main for all five.assert.partialDeepStrictEqual(last item of "Not in this PR").A second review, of the folded diff, is not done yet. The PR stays a draft until it is.