Skip to content

Commit

Permalink
Merge pull request #73386 from CyrusNajmabadi/cleanupParallel
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi authored May 8, 2024
2 parents d4b3936 + de6a2bb commit c0ef482
Show file tree
Hide file tree
Showing 4 changed files with 237 additions and 180 deletions.
98 changes: 18 additions & 80 deletions src/Workspaces/Core/Portable/CodeActions/CodeAction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@
using System.ComponentModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CaseCorrection;
using Microsoft.CodeAnalysis.CodeCleanup;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CodeRefactorings;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Internal.Log;
Expand All @@ -32,7 +29,7 @@ namespace Microsoft.CodeAnalysis.CodeActions;
/// <summary>
/// An action produced by a <see cref="CodeFixProvider"/> or a <see cref="CodeRefactoringProvider"/>.
/// </summary>
public abstract class CodeAction
public abstract partial class CodeAction
{
private static readonly Dictionary<Type, bool> s_isNonProgressGetChangedSolutionAsyncOverridden = [];
private static readonly Dictionary<Type, bool> s_isNonProgressComputeOperationsAsyncOverridden = [];
Expand Down Expand Up @@ -248,7 +245,7 @@ private protected virtual async Task<ImmutableArray<CodeActionOperation>> GetOpe

if (operations != null)
{
return await this.PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
return await PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
}

return [];
Expand All @@ -269,7 +266,7 @@ internal async Task<ImmutableArray<CodeActionOperation>> GetPreviewOperationsAsy

if (operations != null)
{
return await this.PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
return await PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
}

return [];
Expand Down Expand Up @@ -408,7 +405,7 @@ protected virtual Task<Document> GetChangedDocumentAsync(IProgress<CodeAnalysisP
return solution;
}

return await this.PostProcessChangesAsync(originalSolution, solution, cancellationToken).ConfigureAwait(false);
return await PostProcessChangesAsync(originalSolution, solution, cancellationToken).ConfigureAwait(false);
}

internal Task<Document> GetChangedDocumentInternalAsync(CancellationToken cancellation)
Expand All @@ -420,19 +417,21 @@ internal Task<Document> GetChangedDocumentInternalAsync(CancellationToken cancel
/// <param name="operations">A list of operations.</param>
/// <param name="cancellationToken">A cancellation token.</param>
/// <returns>A new list of operations with post processing steps applied to any <see cref="ApplyChangesOperation"/>'s.</returns>
#pragma warning disable CA1822 // Mark members as static. This is a public API.
protected Task<ImmutableArray<CodeActionOperation>> PostProcessAsync(IEnumerable<CodeActionOperation> operations, CancellationToken cancellationToken)
=> PostProcessAsync(originalSolution: null!, operations, cancellationToken);
#pragma warning restore CA1822 // Mark members as static
=> PostProcessAsync(originalSolution: null, operations, cancellationToken);

internal async Task<ImmutableArray<CodeActionOperation>> PostProcessAsync(
Solution originalSolution, IEnumerable<CodeActionOperation> operations, CancellationToken cancellationToken)
internal static async Task<ImmutableArray<CodeActionOperation>> PostProcessAsync(
Solution? originalSolution, IEnumerable<CodeActionOperation> operations, CancellationToken cancellationToken)
{
using var result = TemporaryArray<CodeActionOperation>.Empty;

foreach (var op in operations)
{
if (op is ApplyChangesOperation ac)
{
result.Add(new ApplyChangesOperation(await this.PostProcessChangesAsync(originalSolution, ac.ChangedSolution, cancellationToken).ConfigureAwait(false)));
result.Add(new ApplyChangesOperation(await PostProcessChangesAsync(originalSolution, ac.ChangedSolution, cancellationToken).ConfigureAwait(false)));
}
else
{
Expand All @@ -448,11 +447,13 @@ internal async Task<ImmutableArray<CodeActionOperation>> PostProcessAsync(
/// </summary>
/// <param name="changedSolution">The solution changed by the <see cref="CodeAction"/>.</param>
/// <param name="cancellationToken">A cancellation token</param>
#pragma warning disable CA1822 // Mark members as static. This is a public API.
protected Task<Solution> PostProcessChangesAsync(Solution changedSolution, CancellationToken cancellationToken)
=> PostProcessChangesAsync(originalSolution: null!, changedSolution, cancellationToken);
#pragma warning restore CA1822 // Mark members as static
=> PostProcessChangesAsync(originalSolution: null, changedSolution, cancellationToken);

internal async Task<Solution> PostProcessChangesAsync(
Solution originalSolution,
private static async Task<Solution> PostProcessChangesAsync(
Solution? originalSolution,
Solution changedSolution,
CancellationToken cancellationToken)
{
Expand All @@ -461,38 +462,10 @@ internal async Task<Solution> PostProcessChangesAsync(
// underneath us). But it's the only option we have for the compat case with existing public extension
// points.
originalSolution ??= changedSolution.Workspace.CurrentSolution;
var solutionChanges = changedSolution.GetChanges(originalSolution);

var processedSolution = changedSolution;

// process changed projects
foreach (var projectChanges in solutionChanges.GetProjectChanges())
{
var documentsToProcess = projectChanges.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat(
projectChanges.GetAddedDocuments());

foreach (var documentId in documentsToProcess)
{
var document = processedSolution.GetRequiredDocument(documentId);
var processedDocument = await PostProcessChangesAsync(document, cancellationToken).ConfigureAwait(false);
processedSolution = processedDocument.Project.Solution;
}
}

// process completely new projects too
foreach (var addedProject in solutionChanges.GetAddedProjects())
{
var documentsToProcess = addedProject.DocumentIds;

foreach (var documentId in documentsToProcess)
{
var document = processedSolution.GetRequiredDocument(documentId);
var processedDocument = await PostProcessChangesAsync(document, cancellationToken).ConfigureAwait(false);
processedSolution = processedDocument.Project.Solution;
}
}

return processedSolution;
var globalOptions = changedSolution.Services.GetService<ILegacyGlobalCleanCodeGenerationOptionsWorkspaceService>();
var fallbackOptions = globalOptions?.Provider ?? CodeActionOptions.DefaultProvider;
return await CleanSyntaxAndSemanticsAsync(originalSolution, changedSolution, fallbackOptions, CodeAnalysisProgress.None, cancellationToken).ConfigureAwait(false);
}

/// <summary>
Expand All @@ -518,41 +491,6 @@ protected virtual async Task<Document> PostProcessChangesAsync(Document document
return document;
}

internal static async Task<Document> CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
// First, do a syntax pass. Ensuring that things are formatted correctly based on the original nodes and
// tokens. We want to do this prior to cleaning semantics as semantic cleanup can change the shape of the tree
// (for example, by removing tokens), which can then cause formatting to not work as expected.
var document1 = await CleanupSyntaxAsync(document, options, cancellationToken).ConfigureAwait(false);

// Now, do the semantic cleaning pass.
var document2 = await CleanupSemanticsAsync(document1, options, cancellationToken).ConfigureAwait(false);
return document2;
}

internal static async Task<Document> CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
// format any node with explicit formatter annotation
var document1 = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);

// format any elastic whitespace
var document2 = await Formatter.FormatAsync(document1, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);
return document2;
}

internal static async Task<Document> CleanupSemanticsAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
var document1 = await ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken).ConfigureAwait(false);
var document2 = await Simplifier.ReduceAsync(document1, Simplifier.Annotation, options.SimplifierOptions, cancellationToken).ConfigureAwait(false);
var document3 = await CaseCorrector.CaseCorrectAsync(document2, CaseCorrector.Annotation, cancellationToken).ConfigureAwait(false);

// After doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a good state.
// The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be cleaned.
var document4 = await CleanupSyntaxAsync(document3, options, cancellationToken).ConfigureAwait(false);

return document4;
}

#region Factories for standard code actions

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public abstract class CodeActionWithOptions : CodeAction

if (operations != null)
{
operations = await this.PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
operations = await PostProcessAsync(originalSolution, operations, cancellationToken).ConfigureAwait(false);
}

return operations;
Expand Down
180 changes: 180 additions & 0 deletions src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
// Licensed to the .NET Foundation under one or more agreements.
// 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.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CaseCorrection;
using Microsoft.CodeAnalysis.CodeCleanup;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Remote;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Microsoft.CodeAnalysis.Simplification;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CodeActions;

public abstract partial class CodeAction
{
/// <summary>
/// We do cleanup in N serialized passes. This allows us to process all documents in parallel, while only forking
/// the solution N times *total* (instead of N times *per* document).
/// </summary>
private static readonly ImmutableArray<Func<Document, CodeCleanupOptions, CancellationToken, Task<Document>>> s_cleanupPasses =
[
// First, ensure that everything is formatted as the feature asked for. We want to do this prior to doing
// semantic cleanup as the semantic cleanup passes may end up making changes that end up dropping some of
// the formatting/elastic annotations that the feature wanted respected.
static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken),
// Then add all missing imports to all changed documents.
static (document, options, cancellationToken) => ImportAdder.AddImportsFromSymbolAnnotationAsync(document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken),
// Then simplify any expanded constructs.
static (document, options, cancellationToken) => Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken),
// The do any necessary case correction for VB files.
static (document, options, cancellationToken) => CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken),
// Finally, after doing the semantic cleanup, do another syntax cleanup pass to ensure that the tree is in a
// good state. The semantic cleanup passes may have introduced new nodes with elastic trivia that have to be
// cleaned.
static (document, options, cancellationToken) => CleanupSyntaxAsync(document, options, cancellationToken),
];

private static async Task<Document> CleanupSyntaxAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
Contract.ThrowIfFalse(document.SupportsSyntaxTree);

// format any node with explicit formatter annotation
var document1 = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);

// format any elastic whitespace
var document2 = await Formatter.FormatAsync(document1, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);
return document2;
}

internal static ImmutableArray<DocumentId> GetAllChangedOrAddedDocumentIds(
Solution originalSolution,
Solution changedSolution)
{
var solutionChanges = changedSolution.GetChanges(originalSolution);
var documentIds = solutionChanges
.GetProjectChanges()
.SelectMany(p => p.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat(p.GetAddedDocuments()))
.Concat(solutionChanges.GetAddedProjects().SelectMany(p => p.DocumentIds))
.ToImmutableArray();
return documentIds;
}

internal static async Task<Solution> CleanSyntaxAndSemanticsAsync(
Solution originalSolution,
Solution changedSolution,
CodeCleanupOptionsProvider optionsProvider,
IProgress<CodeAnalysisProgress> progress,
CancellationToken cancellationToken)
{
var documentIds = GetAllChangedOrAddedDocumentIds(originalSolution, changedSolution);
var documentIdsAndOptionsToClean = await GetDocumentIdsAndOptionsToCleanAsync().ConfigureAwait(false);

// Then do a pass where we cleanup semantics.
var cleanedSolution = await RunAllCleanupPassesInOrderAsync(
changedSolution, documentIdsAndOptionsToClean, progress, cancellationToken).ConfigureAwait(false);

return cleanedSolution;

async Task<ImmutableArray<(DocumentId documentId, CodeCleanupOptions codeCleanupOptions)>> GetDocumentIdsAndOptionsToCleanAsync()
{
using var _ = ArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>.GetInstance(documentIds.Length, out var documentIdsAndOptions);
foreach (var documentId in documentIds)
{
var document = changedSolution.GetRequiredDocument(documentId);

// Only care about documents that support syntax. Non-C#/VB files can't be cleaned.
if (document.SupportsSyntaxTree)
{
var codeActionOptions = await document.GetCodeCleanupOptionsAsync(optionsProvider, cancellationToken).ConfigureAwait(false);
documentIdsAndOptions.Add((documentId, codeActionOptions));
}
}

return documentIdsAndOptions.ToImmutableAndClear();
}
}

internal static async ValueTask<Document> CleanupDocumentAsync(Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
if (!document.SupportsSyntaxTree)
return document;

var cleanedSolution = await RunAllCleanupPassesInOrderAsync(
document.Project.Solution,
[(document.Id, options)],
CodeAnalysisProgress.None,
cancellationToken).ConfigureAwait(false);

return cleanedSolution.GetRequiredDocument(document.Id);
}

private static async Task<Solution> RunAllCleanupPassesInOrderAsync(
Solution solution,
ImmutableArray<(DocumentId documentId, CodeCleanupOptions options)> documentIdsAndOptions,
IProgress<CodeAnalysisProgress> progress,
CancellationToken cancellationToken)
{
// One item per document per cleanup pass.
progress.AddItems(documentIdsAndOptions.Length * s_cleanupPasses.Length);

var currentSolution = solution;
foreach (var cleanupPass in s_cleanupPasses)
currentSolution = await RunParallelCleanupPassAsync(currentSolution, cleanupPass).ConfigureAwait(false);

return currentSolution;

async Task<Solution> RunParallelCleanupPassAsync(
Solution solution, Func<Document, CodeCleanupOptions, CancellationToken, Task<Document>> cleanupDocumentAsync)
{
// We're about to making a ton of calls to this new solution, including expensive oop calls to get up to
// date compilations, skeletons and SG docs. Create and pin this solution so that all remote calls operate
// on the same fork and do not cause the forked solution to be created and dropped repeatedly.
using var _ = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false);

return await ProducerConsumer<(DocumentId documentId, SyntaxNode newRoot)>.RunParallelAsync(
source: documentIdsAndOptions,
produceItems: static async (documentIdAndOptions, callback, args, cancellationToken) =>
{
// As we finish each document, update our progress.
using var _ = args.progress.ItemCompletedScope();
var (documentId, options) = documentIdAndOptions;
// Fetch the current state of the document from this fork of the solution.
var document = args.solution.GetRequiredDocument(documentId);
Contract.ThrowIfFalse(document.SupportsSyntaxTree, "GetDocumentIdsAndOptionsAsync should only be returning documents that support syntax");
// Now, perform the requested cleanup pass on it.
var cleanedDocument = await args.cleanupDocumentAsync(document, options, cancellationToken).ConfigureAwait(false);
if (cleanedDocument is null || cleanedDocument == document)
return;
// Now get the cleaned root and pass it back to the consumer.
var newRoot = await cleanedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
callback((documentId, newRoot));
},
consumeItems: static async (stream, args, cancellationToken) =>
{
// Grab all the cleaned roots and produce the new solution snapshot from that.
var currentSolution = args.solution;
await foreach (var (documentId, newRoot) in stream)
currentSolution = currentSolution.WithDocumentSyntaxRoot(documentId, newRoot);
return currentSolution;
},
args: (solution, progress, cleanupDocumentAsync),
cancellationToken).ConfigureAwait(false);
}
}
}
Loading

0 comments on commit c0ef482

Please sign in to comment.