Skip to content

🐇 Avoid computing unnecessary semantic models#43566

Merged
sharwell merged 2 commits intodotnet:masterfrom
sharwell:faster-import-adder
Apr 25, 2020
Merged

🐇 Avoid computing unnecessary semantic models#43566
sharwell merged 2 commits intodotnet:masterfrom
sharwell:faster-import-adder

Conversation

@sharwell
Copy link
Contributor

@sharwell sharwell commented Apr 22, 2020

Significantly improves the performance BatchFixAllProvider when imports do not need to be added.

For one sample project, the Fix All in Solution performance for Roslyn.sln dropped from over one hour (cancelled before completion) to less than 30 seconds.

@sharwell sharwell requested a review from a team as a code owner April 22, 2020 19:23
@sharwell sharwell changed the title Avoid computing unnecessary semantic models 🐇 Avoid computing unnecessary semantic models Apr 22, 2020
{
var importsToAdd = ArrayBuilder<SyntaxNode>.GetInstance();

var model = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is always going to do this work (and pass back out) , i don't get why it woudln't be nicer to just have the caller compute this andpass in.

Copy link
Contributor Author

@sharwell sharwell Apr 22, 2020

Choose a reason for hiding this comment

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

It pushes a bunch of implementation knowledge up to the caller. The deconstruction assignment and switch expression would both be eliminated. This call would need to compute in advance, while the other call does not compute in advance but might compute in the implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can implement this change if you want to see it. I felt like the additional noise didn't make it worth it.

Copy link
Contributor

Choose a reason for hiding this comment

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

it just feels weird to have it not passed in, always computed, and passed out so the caller wil have it...

@sharwell sharwell merged commit 212100b into dotnet:master Apr 25, 2020
@ghost ghost added this to the Next milestone Apr 25, 2020
@sharwell sharwell deleted the faster-import-adder branch April 25, 2020 21:08
@sharwell sharwell modified the milestones: Next, temp, 16.7.P1 Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants