Skip to content

[Remangler] Improve performance by caching hashes. #72874

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

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Apr 5, 2024

The deepHash() function gets called repeatedly as we descend the node tree, which results in O(n^2) behaviour because we're traversing entire node subtree from each node we try substitution in, in order to calculate the hash.

Fix by adding a hash table for hashes, so that we can look up hashes we've already computed.

This appears to yield a 26.8% saving in local tests.

rdar://125739630

The deepHash() function gets called repeatedly as we descend the
node tree, which results in O(n^2) behaviour because we're traversing
entire node subtree from each node we try substitution in, in order
to calculate the hash.

Fix by adding a hash table for hashes, so that we can look up hashes
we've already computed.

This appears to yield a 26.8% saving in local tests.

rdar://125739630
@al45tair al45tair added 🍒 release cherry pick Flag: Release branch cherry picks swift 6.0 labels Apr 5, 2024
@al45tair al45tair requested a review from a team as a code owner April 5, 2024 17:09
@al45tair
Copy link
Contributor Author

al45tair commented Apr 5, 2024

Explanation: The remangler was traversing the node tree over and over again computing hashes, while iterating the node tree; this is both O(n²) behaviour and actually we're doing unnecessary work. This PR adds a cache for the node hashes, which lets us short-circuit all of the extra work we were doing.
Original PR: #72835
Reviewed by: @drexin @w6sec
Risk: This change is entirely internal to the remangler, and just affects its performance, not its output.
Resolves: rdar://125739630
Tests: We've run performance tests on this code to verify that this improves matters. There are fairly extensive tests of the demangler and remangler that also still pass after this change.

@al45tair
Copy link
Contributor Author

al45tair commented Apr 8, 2024

@swift-ci Please test

@al45tair al45tair merged commit b42f4a7 into swiftlang:release/6.0 Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants