perf: Remove expensive call to find matching in children #66
Conversation
This file contains 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
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
jpedroh
added a commit
that referenced
this pull request
Jul 27, 2024
…heck (#67) Sure, here's the revised version with the term "auxiliary HashMap": This work follows up on #66. Currently, the Matchings API provides a quick way to retrieve a matching entry for two arbitrary nodes. However, a common use case during the merge process involves finding a matching entry for a node without prior knowledge of which other node it matches with. While we have an API for this scenario, it is inefficient as it may iterate over all matchings in the worst case. This PR introduces an enhancement to address this inefficiency. The approach involves storing an auxiliary HashMap where both the key and value are nodes, say A and B. This way, there is an entry in the HashMap only if there is a matching between node A and B. This allows us to quickly check if a matching for node A exists by simply checking if an entry is present in the HashMap. This change reduces the lookup time to O(1), at the cost of additional memory usage and some performance overhead in building and updating this auxiliary HashMap, which were found to be negligible compared to the performance gains achieved.
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.
During the merge process, given two nodes, left and right, we need to iterate over their children to determine how to merge them. In each iteration, we try to find a match for the current left child among the right children and vice versa. Previously, we were examining all captured matches across the entire program tree, which was heavily inefficient.
This PR changes the implementation to only consider the matches captured in the other node's children, significantly reducing the number of iterations required. Initial observations in empirical scenarios showed a performance boost of nearly 3x.