Skip to content
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

Performance fix for AnalysisEntityMapAbstractDomain.Merge algorithm #3058

Merged
merged 6 commits into from Nov 22, 2019

Conversation

mavasani
Copy link
Member

We now ensure that we only create new entries in the result map if the entity key is a valid tracked entity

We now ensure that we only create new entries in the result map if the entity key is a valid tracked entity
@mavasani mavasani requested a review from a team as a code owner November 21, 2019 13:21
… with this fix. Working on addressing these failures in a follow-up commit.
… that new reachable entities from Merge algorithm are not skipped. This commit fixes the analysis for tests added in previous commit.
@mavasani
Copy link
Member Author

@dotpaul I was able to validate no performance regression in building Roslyn after this fix, let me know once your validations are done. Thanks!

@dotpaul
Copy link
Contributor

dotpaul commented Nov 22, 2019

Other than the one case @LLLXXXCCC emailed about, the previous repros looks good to me! I'm trying a pass on a larger code base; will approve after that finishes.

Copy link
Contributor

@dotpaul dotpaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 Improved performance from what I can see.

@mavasani
Copy link
Member Author

Thank you @dotpaul!

@mavasani mavasani merged commit 1a63d89 into dotnet:2.9.x Nov 22, 2019
@mavasani mavasani deleted the PerfFixes branch November 22, 2019 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants