From 8e914f954ceb956fca3cb6554a19a903abd6be5f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 23 Jan 2025 23:30:35 -0800 Subject: [PATCH 01/13] Speed up 'fix all' for 'use auto prop' by running in parallel --- .../UseAutoPropertyFixAllProvider.cs | 67 ++++++++++++++++--- 1 file changed, 56 insertions(+), 11 deletions(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index 333052f575da3..a9aba0c43cd61 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -8,6 +8,8 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CodeFixesAndRefactorings; +using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Shared.Utilities; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.UseAutoProperty; @@ -34,23 +36,66 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP // effectively apply each fix one at a time, moving the solution forward each time. As we process each // diagnostic, we attempt to re-recover the field/property it was referring to in the original solution to // the current solution. + // + // Note: we can process each project in parallel. That's because all changes to a field/prop only impact + // the project they are in, and nothing beyond that. var originalSolution = originalContext.Solution; - var currentSolution = originalSolution; - foreach (var currentContext in contexts) - { - var documentToDiagnostics = await FixAllContextHelper.GetDocumentDiagnosticsToFixAsync(currentContext).ConfigureAwait(false); - foreach (var (_, diagnostics) in documentToDiagnostics) + // Add a progress item for each context we need to process. + originalContext.Progress.AddItems(contexts.Length); + + var finalSolution = await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( + contexts, + produceItems: async static (currentContext, callback, args, cancellationToken) => { - foreach (var diagnostic in diagnostics) + // Within a single context (a project) get all diagnostics, and then handle each diagnostic, one at + // a time, to get the final state of the project. + var (originalContext, provider) = args; + + // Complete a progress item as we finish each project. + using var _ = originalContext.Progress.ItemCompletedScope(); + + var originalSolution = originalContext.Solution; + var currentSolution = originalSolution; + + var documentToDiagnostics = await FixAllContextHelper.GetDocumentDiagnosticsToFixAsync(currentContext).ConfigureAwait(false); + foreach (var (document, diagnostics) in documentToDiagnostics) + { + foreach (var diagnostic in diagnostics) + { + currentSolution = await provider.ProcessResultAsync( + originalSolution, currentSolution, diagnostic, cancellationToken).ConfigureAwait(false); + } + } + + // After we finish this context, report the changed documents to the consumeItems callback to process. + // This also lets us release all the forked solution info we created above. + foreach (var projectChanges in currentSolution.GetChanges(originalSolution).GetProjectChanges()) { - currentSolution = await provider.ProcessResultAsync( - originalSolution, currentSolution, diagnostic, cancellationToken).ConfigureAwait(false); + foreach (var changedDocumentId in projectChanges.GetChangedDocuments()) + { + var changedDocument = currentSolution.GetRequiredDocument(changedDocumentId); + var changedRoot = await changedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + callback((changedDocumentId, changedRoot)); + } } - } - } + }, + consumeItems: async static (documentsIdsAndNewRoots, args, cancellationToken) => + { + var (originalContext, provider) = args; + var originalSolution = originalContext.Solution; + var currentSolution = originalSolution; + + // Take all the changed documents and update the final solution with them. + await foreach (var (documentId, newRoot) in documentsIdsAndNewRoots) + currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); + + return currentSolution; + }, + args: (originalContext, provider), + cancellationToken).ConfigureAwait(false); - return currentSolution; + return finalSolution; } } } From 8d77f7fbd2494f049b1ae00eb3ce0e4affbdd851 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 23 Jan 2025 23:46:28 -0800 Subject: [PATCH 02/13] Add error reporting --- .../AbstractUseAutoPropertyCodeFixProvider.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs b/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs index 52f02f4950ab6..841532b9e395e 100644 --- a/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; @@ -13,6 +14,7 @@ using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Formatting.Rules; using Microsoft.CodeAnalysis.LanguageService; @@ -85,6 +87,19 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) private async Task ProcessResultAsync( Solution originalSolution, Solution currentSolution, Diagnostic diagnostic, CancellationToken cancellationToken) + { + try + { + return await ProcessResultWorkerAsync(originalSolution, currentSolution, diagnostic, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (FatalError.ReportAndCatchUnlessCanceled(ex)) + { + return currentSolution; + } + } + + private async Task ProcessResultWorkerAsync( + Solution originalSolution, Solution currentSolution, Diagnostic diagnostic, CancellationToken cancellationToken) { var (field, property) = await MapDiagnosticToCurrentSolutionAsync( diagnostic, originalSolution, currentSolution, cancellationToken).ConfigureAwait(false); From 34a1414e6ba9368330e191f3b2909084d1e01da5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 00:33:18 -0800 Subject: [PATCH 03/13] Update src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs --- .../Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index a9aba0c43cd61..2d993566df7db 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -59,7 +59,7 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP var currentSolution = originalSolution; var documentToDiagnostics = await FixAllContextHelper.GetDocumentDiagnosticsToFixAsync(currentContext).ConfigureAwait(false); - foreach (var (document, diagnostics) in documentToDiagnostics) + foreach (var (_, diagnostics) in documentToDiagnostics) { foreach (var diagnostic in diagnostics) { From eb3b07f721d25965e601d3b3adc97a31993337c1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 00:34:44 -0800 Subject: [PATCH 04/13] Apply suggestions from code review --- .../UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index 2d993566df7db..e03c6054ee17d 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -82,9 +82,8 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP }, consumeItems: async static (documentsIdsAndNewRoots, args, cancellationToken) => { - var (originalContext, provider) = args; - var originalSolution = originalContext.Solution; - var currentSolution = originalSolution; + var (originalContext, _) = args; + var currentSolution = originalContext.Solution; // Take all the changed documents and update the final solution with them. await foreach (var (documentId, newRoot) in documentsIdsAndNewRoots) From e9da589c80f42b8ac07005ae976ef6a989d0d9d3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 00:35:18 -0800 Subject: [PATCH 05/13] Apply suggestions from code review --- .../Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index e03c6054ee17d..5cb1a68be14a5 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -82,8 +82,7 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP }, consumeItems: async static (documentsIdsAndNewRoots, args, cancellationToken) => { - var (originalContext, _) = args; - var currentSolution = originalContext.Solution; + var currentSolution = args.originalContext.Solution; // Take all the changed documents and update the final solution with them. await foreach (var (documentId, newRoot) in documentsIdsAndNewRoots) From 15f06a237553878e219e526c4d9df90e596d6be4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 00:36:57 -0800 Subject: [PATCH 06/13] Apply suggestions from code review --- .../Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index 5cb1a68be14a5..6f934f7467c45 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -44,7 +44,7 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP // Add a progress item for each context we need to process. originalContext.Progress.AddItems(contexts.Length); - var finalSolution = await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( + return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( contexts, produceItems: async static (currentContext, callback, args, cancellationToken) => { @@ -93,7 +93,6 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP args: (originalContext, provider), cancellationToken).ConfigureAwait(false); - return finalSolution; } } } From 42a384fbbe5f1d1d5ee45328b7a385cd6c5a3cee Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 00:37:15 -0800 Subject: [PATCH 07/13] Update src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs --- .../Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index 6f934f7467c45..d0c04b3e596e9 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -92,7 +92,6 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP }, args: (originalContext, provider), cancellationToken).ConfigureAwait(false); - } } } From 7f21472b97e3206667b0152136ffa05d49fb83b1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 00:39:02 -0800 Subject: [PATCH 08/13] Tweak --- .../UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index a9aba0c43cd61..504336be7303c 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -30,7 +30,7 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP private async Task FixAllContextsHelperAsync(FixAllContext originalContext, ImmutableArray contexts) { - var cancellationToken = originalContext.CancellationToken; + var cancellationToken = ; // Very slow approach, but the only way we know how to do this correctly and without colliding edits. We // effectively apply each fix one at a time, moving the solution forward each time. As we process each @@ -44,7 +44,7 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP // Add a progress item for each context we need to process. originalContext.Progress.AddItems(contexts.Length); - var finalSolution = await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( + return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( contexts, produceItems: async static (currentContext, callback, args, cancellationToken) => { @@ -93,9 +93,7 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP return currentSolution; }, args: (originalContext, provider), - cancellationToken).ConfigureAwait(false); - - return finalSolution; + originalContext.CancellationToken).ConfigureAwait(false); } } } From b1799b741fb4265e38b21f5a90302a809fb0b89a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 00:44:05 -0800 Subject: [PATCH 09/13] Simplify --- .../Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index 82d7c727ad977..ca7789372a101 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -79,9 +79,8 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP }, consumeItems: async static (documentsIdsAndNewRoots, args, cancellationToken) => { + // Now take all the changed documents and produce the final solution with all of them combined. var currentSolution = args.originalContext.Solution; - - // Take all the changed documents and update the final solution with them. await foreach (var (documentId, newRoot) in documentsIdsAndNewRoots) currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); From 8f4e4ffe6d77c0511bd27c2e282bea203eb93ce9 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 00:45:23 -0800 Subject: [PATCH 10/13] Simplify --- .../UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index ca7789372a101..9d5383c85f64e 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -67,14 +67,11 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP // After we finish this context, report the changed documents to the consumeItems callback to process. // This also lets us release all the forked solution info we created above. - foreach (var projectChanges in currentSolution.GetChanges(originalSolution).GetProjectChanges()) + foreach (var changedDocumentId in originalSolution.GetChangedDocuments(currentSolution)) { - foreach (var changedDocumentId in projectChanges.GetChangedDocuments()) - { - var changedDocument = currentSolution.GetRequiredDocument(changedDocumentId); - var changedRoot = await changedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - callback((changedDocumentId, changedRoot)); - } + var changedDocument = currentSolution.GetRequiredDocument(changedDocumentId); + var changedRoot = await changedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + callback((changedDocumentId, changedRoot)); } }, consumeItems: async static (documentsIdsAndNewRoots, args, cancellationToken) => From 877732369c53c57e06060ad10e565324c2dfe53b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 00:47:20 -0800 Subject: [PATCH 11/13] Simplify --- .../Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index 9d5383c85f64e..f5693e0a7f720 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections.Generic; using System.Collections.Immutable; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; From 52f788aea0f5ff906d2a4d12694167042085a5a7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 08:51:44 -0800 Subject: [PATCH 12/13] Use WithDocumentSyntaxRoots --- .../UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index f5693e0a7f720..168a05fbd9133 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -76,11 +76,8 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP consumeItems: async static (documentsIdsAndNewRoots, args, cancellationToken) => { // Now take all the changed documents and produce the final solution with all of them combined. - var currentSolution = args.originalContext.Solution; - await foreach (var (documentId, newRoot) in documentsIdsAndNewRoots) - currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot); - - return currentSolution; + return args.originalContext.Solution.WithDocumentSyntaxRoots( + await documentsIdsAndNewRoots.ToImmutableArrayAsync(cancellationToken).ConfigureAwait(false)); }, args: (originalContext, provider), originalContext.CancellationToken).ConfigureAwait(false); From 658496603bed7b568673db151f89430ccd65db7b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Jan 2025 09:21:26 -0800 Subject: [PATCH 13/13] Simplify --- .../UseAutoProperty/UseAutoPropertyFixAllProvider.cs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs index 168a05fbd9133..122e14bff5003 100644 --- a/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/UseAutoPropertyFixAllProvider.cs @@ -40,7 +40,7 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP // Add a progress item for each context we need to process. originalContext.Progress.AddItems(contexts.Length); - return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( + var documentsIdsAndNewRoots = await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync( contexts, produceItems: async static (currentContext, callback, args, cancellationToken) => { @@ -73,14 +73,10 @@ private sealed class UseAutoPropertyFixAllProvider(TProvider provider) : FixAllP callback((changedDocumentId, changedRoot)); } }, - consumeItems: async static (documentsIdsAndNewRoots, args, cancellationToken) => - { - // Now take all the changed documents and produce the final solution with all of them combined. - return args.originalContext.Solution.WithDocumentSyntaxRoots( - await documentsIdsAndNewRoots.ToImmutableArrayAsync(cancellationToken).ConfigureAwait(false)); - }, args: (originalContext, provider), originalContext.CancellationToken).ConfigureAwait(false); + + return originalContext.Solution.WithDocumentSyntaxRoots(documentsIdsAndNewRoots); } } }