-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Update code cleanup to run in parallel #73368
Update code cleanup to run in parallel #73368
Conversation
solution.Workspace, | ||
(progress, cancellationToken) => FixProjectsAsync( | ||
_globalOptions, solution, solution.Projects.Where(p => p.SupportsCompilation).ToImmutableArray(), context.EnabledFixIds, progress, cancellationToken), | ||
context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just inlined the local function version. Note: FixSolution became FixProjects where the set of projects to fix is passed in.
// Just defer to FixProjectsAsync, passing in all fixable projects in the solution. | ||
(progress, cancellationToken) => FixProjectsAsync( | ||
_globalOptions, solution, solution.Projects.Where(p => p.SupportsCompilation).ToImmutableArray(), context.EnabledFixIds, progress, cancellationToken), | ||
context).ConfigureAwait(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inlined this method.
// Just defer to FixProjectsAsync, passing in this single project to fix. | ||
(progress, cancellationToken) => FixProjectsAsync( | ||
_globalOptions, project.Solution, [project], context.EnabledFixIds, progress, cancellationToken), | ||
context).ConfigureAwait(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inlined this method.
var newDocument = await FixDocumentAsync(document, context.EnabledFixIds, progress, options, cancellationToken).ConfigureAwait(true); | ||
return newDocument.Project.Solution; | ||
}, | ||
context).ConfigureAwait(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inlined this methodl
// FixDocumentAsync reports progress within a document, but we only want to report progress at | ||
// the document granularity. So we pass CodeAnalysisProgress.None here so that inner progress | ||
// updates don't affect us. | ||
var fixedDocument = await FixDocumentAsync(document, args.enabledFixIds, CodeAnalysisProgress.None, ideOptions, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that this logic is different from before. now we operate in parallel, always processing documents from the original solution (previously we would process a document, fork the solution, and repeat). This was obviously insanely slow.
await foreach (var (documentId, newRoot) in stream) | ||
currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); | ||
|
||
return currentSolution; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now we can apply all the changes in one go to the solution, producing the final solution in one shot.
@ToddGrun ptal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasonmalinowski For review when you get back. |
Saves >15 minutes running this on roslyn.