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.

Sign up for GitHub

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

Jump to bottom

SolutionState.Branch does a bunch of unnecessary project enumeration #53769

Closed
jasonmalinowski opened this issue May 29, 2021 · 1 comment · Fixed by #54196
Closed

SolutionState.Branch does a bunch of unnecessary project enumeration #53769

jasonmalinowski opened this issue May 29, 2021 · 1 comment · Fixed by #54196
Assignees
@sharwell @mavasani
Labels
Area-IDE Area-Performance Concept-Continuous Improvement Performance-Scenario-Solution-Close This issue affects solution close performance. Performance-Scenario-Solution-Open This issue affects solution open performance. Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Milestone
17.0.P2

Comments

@jasonmalinowski
Copy link
Member

This line is showing up as a hot path in solution open/close:

roslyn/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs

Line 223 in f4c808d

options ??= Options.WithLanguages(GetRemoteSupportedProjectLanguages(idToProjectStateMap));

Because on almost every workspace change, we are re-enumerating all projects and tossing the common languages into a hash set, even if no project was added/removed. At least one trace is showing this is maybe 5% of Roslyn's contribution to solution close and 15% (!) we spend updating our workspace in solution open.
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels May 29, 2021
@jasonmalinowski
Copy link
Member Author

FYI to @mavasani and @davkean.

@jinujoseph jinujoseph added Area-Performance and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 1, 2021
@jinujoseph jinujoseph added this to the 16.11 milestone Jun 1, 2021
mavasani added a commit to mavasani/roslyn that referenced this issue Jun 1, 2021
@mavasani
Avoid unnecessary project enumeration in SolutionState.Branch
47c3c7b 
Fixes dotnet#53769
@mavasani mavasani mentioned this issue Jun 1, 2021
Closed
@panopticoncentral panopticoncentral added Performance-Scenario-Solution-Open This issue affects solution open performance. Performance-Scenario-Solution-Close This issue affects solution close performance. labels Jun 2, 2021
@sharwell sharwell mentioned this issue Jun 17, 2021
Closed
@sharwell sharwell added the 4 - In Review A fix for the issue is submitted for review. label Jun 17, 2021
@sharwell sharwell mentioned this issue Jun 18, 2021
Merged
@sharwell sharwell self-assigned this Jun 18, 2021
sharwell added a commit to sharwell/roslyn that referenced this issue Jun 18, 2021
@sharwell
Optimize calculation of remote supported languages
e7ac5ba 
Fixes dotnet#53769
@sharwell sharwell added Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented and removed 4 - In Review A fix for the issue is submitted for review. labels Jun 18, 2021
@sharwell sharwell modified the milestones: 16.11, Next Jun 18, 2021
@RikkiGibson RikkiGibson modified the milestones: Next, 17.0.P2 Jun 29, 2021
@jasonmalinowski jasonmalinowski mentioned this issue Nov 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sharwell sharwell

@mavasani mavasani

Labels
Area-IDE Area-Performance Concept-Continuous Improvement Performance-Scenario-Solution-Close This issue affects solution close performance. Performance-Scenario-Solution-Open This issue affects solution open performance. Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Milestone
17.0.P2
Development

Successfully merging a pull request may close this issue.

Optimize calculation of remote supported languages sharwell/roslyn
6 participants
@jasonmalinowski @panopticoncentral @sharwell @RikkiGibson @mavasani @jinujoseph