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.

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

Use compile-time solution for solution crawler and EnC #52537

Merged
merged 7 commits into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Internal.Log;
using Microsoft.CodeAnalysis.Shared.TestHooks;
Expand Down Expand Up @@ -306,7 +307,8 @@ public Registration(int correlationId, Workspace workspace, SolutionCrawlerProgr
ProgressReporter = progressReporter;
}

public Solution CurrentSolution => Workspace.CurrentSolution;
tmat marked this conversation as resolved.
Show resolved Hide resolved
public Solution GetSolutionToAnalyze()
=> Workspace.Services.GetRequiredService<ICompileTimeSolutionProvider>().GetCurrentCompileTimeSolution();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ protected override async Task ExecuteAsync()
// see whether we have work item for the document
Contract.ThrowIfFalse(GetNextWorkItem(out var workItem, out var documentCancellation));

var solution = _processor.CurrentSolution;
var solution = _processor._registration.GetSolutionToAnalyze();

// okay now we have work to do
await ProcessDocumentAsync(solution, Analyzers, workItem, documentCancellation).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,7 @@ public void Shutdown()

public ImmutableArray<IIncrementalAnalyzer> Analyzers => _normalPriorityProcessor.Analyzers;

private Solution CurrentSolution => _registration.CurrentSolution;
private ProjectDependencyGraph DependencyGraph => CurrentSolution.GetProjectDependencyGraph();
private ProjectDependencyGraph DependencyGraph => _registration.GetSolutionToAnalyze().GetProjectDependencyGraph();
private IDiagnosticAnalyzerService? DiagnosticAnalyzerService => _lazyDiagnosticAnalyzerService?.Value;

public Task AsyncProcessorTask
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ private async Task ProcessProjectAsync(ImmutableArray<IIncrementalAnalyzer> anal
// we do have work item for this project
var projectId = workItem.ProjectId;
var processedEverything = false;
var processingSolution = Processor.CurrentSolution;
var processingSolution = Processor._registration.GetSolutionToAnalyze();

try
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ private async Task ProcessDocumentAsync(ImmutableArray<IIncrementalAnalyzer> ana
// using later version of solution is always fine since, as long as there is new work item in the queue,
// solution crawler will eventually call the last workitem with the lastest solution
// making everything to catch up
var solution = Processor.CurrentSolution;
var solution = Processor._registration.GetSolutionToAnalyze();
try
{
using (Logger.LogBlock(FunctionId.WorkCoordinator_ProcessDocumentAsync, w => w.ToString(), workItem, cancellationToken))
Expand Down Expand Up @@ -522,7 +522,10 @@ private async Task ResetStatesAsync()
return;
}

await Processor.RunAnalyzersAsync(Analyzers, Processor.CurrentSolution, workItem: new WorkItem(), (a, s, c) => a.NewSolutionSnapshotAsync(s, c), CancellationToken).ConfigureAwait(false);
await Processor.RunAnalyzersAsync(
Analyzers,
Processor._registration.GetSolutionToAnalyze(),
workItem: new WorkItem(), (a, s, c) => a.NewSolutionSnapshotAsync(s, c), CancellationToken).ConfigureAwait(false);

foreach (var id in Processor.GetOpenDocumentIds())
{
Expand All @@ -538,7 +541,7 @@ private async Task ResetStatesAsync()

bool IsSolutionChanged()
{
var currentSolution = Processor.CurrentSolution;
var currentSolution = Processor._registration.GetSolutionToAnalyze();
var oldSolution = _lastSolution;

if (currentSolution == oldSolution)
Expand Down Expand Up @@ -577,7 +580,7 @@ public override void Shutdown()
{
base.Shutdown();

SolutionCrawlerLogger.LogIncrementalAnalyzerProcessorStatistics(Processor._registration.CorrelationId, Processor.CurrentSolution, Processor._logAggregator, Analyzers);
SolutionCrawlerLogger.LogIncrementalAnalyzerProcessorStatistics(Processor._registration.CorrelationId, Processor._registration.GetSolutionToAnalyze(), Processor._logAggregator, Analyzers);

_workItemQueue.Dispose();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ protected override async Task ExecuteAsync()

using (data.AsyncToken)
{
var project = _registration.CurrentSolution.GetProject(data.ProjectId);
var project = _registration.GetSolutionToAnalyze().GetProject(data.ProjectId);
if (project == null)
{
return;
Expand All @@ -430,7 +430,7 @@ protected override async Task ExecuteAsync()
}

// do dependency tracking here with current solution
var solution = _registration.CurrentSolution;
var solution = _registration.GetSolutionToAnalyze();
foreach (var projectId in GetProjectsToAnalyze(solution, data.ProjectId))
{
project = solution.GetProject(projectId);
Expand Down
14 changes: 7 additions & 7 deletions src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public WorkCoordinator(
// subscribe to active document changed event for active file background analysis scope.
if (_documentTrackingService != null)
{
_lastActiveDocument = _documentTrackingService.GetActiveDocument(_registration.Workspace.CurrentSolution);
_lastActiveDocument = _documentTrackingService.GetActiveDocument(_registration.GetSolutionToAnalyze());
_documentTrackingService.ActiveDocumentChanged += OnActiveDocumentChanged;
}
}
Expand All @@ -100,7 +100,7 @@ public void AddAnalyzer(IIncrementalAnalyzer analyzer, bool highPriorityForActiv
_documentAndProjectWorkerProcessor.AddAnalyzer(analyzer, highPriorityForActiveFile);

// and ask to re-analyze whole solution for the given analyzer
var scope = new ReanalyzeScope(_registration.CurrentSolution.Id);
var scope = new ReanalyzeScope(_registration.GetSolutionToAnalyze().Id);
Reanalyze(analyzer, scope);
}

Expand Down Expand Up @@ -196,7 +196,7 @@ private void ReanalyzeOnOptionChange(object? sender, OptionChangedEventArgs e)
{
if (forceAnalyze || analyzer.NeedsReanalysisOnOptionChanged(sender, e))
{
var scope = new ReanalyzeScope(_registration.CurrentSolution.Id);
var scope = new ReanalyzeScope(_registration.GetSolutionToAnalyze().Id);
Reanalyze(analyzer, scope);
}
}
Expand All @@ -212,15 +212,15 @@ public void Reanalyze(IIncrementalAnalyzer analyzer, ReanalyzeScope scope, bool
{
// log big reanalysis request from things like fix all, suppress all or option changes
// we are not interested in 1 file re-analysis request which can happen from like venus typing
var solution = _registration.CurrentSolution;
var solution = _registration.GetSolutionToAnalyze();
SolutionCrawlerLogger.LogReanalyze(
CorrelationId, analyzer, scope.GetDocumentCount(solution), scope.GetLanguagesStringForTelemetry(solution), highPriority);
}
}

private void OnActiveDocumentChanged(object? sender, DocumentId activeDocumentId)
{
var solution = _registration.Workspace.CurrentSolution;
var solution = _registration.GetSolutionToAnalyze();

// Check if we are only performing backgroung analysis for active file.
if (activeDocumentId != null)
Expand Down Expand Up @@ -509,7 +509,7 @@ private async Task EnqueueWorkItemAsync(Project project, InvocationReasons invoc

private async Task EnqueueWorkItemAsync(IIncrementalAnalyzer analyzer, ReanalyzeScope scope, bool highPriority)
{
var solution = _registration.CurrentSolution;
var solution = _registration.GetSolutionToAnalyze();
var invocationReasons = highPriority ? InvocationReasons.ReanalyzeHighPriority : InvocationReasons.Reanalyze;

foreach (var document in scope.GetDocuments(solution))
Expand Down Expand Up @@ -683,7 +683,7 @@ internal TestAccessor(WorkCoordinator workCoordinator)

internal void WaitUntilCompletion(ImmutableArray<IIncrementalAnalyzer> workers)
{
var solution = _workCoordinator._registration.CurrentSolution;
var solution = _workCoordinator._registration.GetSolutionToAnalyze();
var list = new List<WorkItem>();

foreach (var project in solution.Projects)
Expand Down
125 changes: 125 additions & 0 deletions src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// 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.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Experiments;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

namespace Microsoft.VisualStudio.LanguageServices
{
/// <summary>
/// Provides a compile-time view of the current workspace solution.
Copy link
Member

Choose a reason for hiding this comment

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

I admit I didn't care for the name "compile-time" because it was ambiguous in precisely this way -- we didn't want to call it "RuntimeSolutionProvider"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't follow. What is it ambiguous with?

/// Workaround for Razor projects which generate both design-time and compile-time source files.
/// TODO: remove https://github.com/dotnet/roslyn/issues/51678
/// </summary>
internal sealed class CompileTimeSolutionProvider : ICompileTimeSolutionProvider
{
[ExportWorkspaceServiceFactory(typeof(ICompileTimeSolutionProvider), WorkspaceKind.Host), Shared]
private sealed class Factory : IWorkspaceServiceFactory
tmat marked this conversation as resolved.
Show resolved Hide resolved
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public Factory()
{
}

[Obsolete(MefConstruction.FactoryMethodMessage, error: true)]
public IWorkspaceService? CreateService(HostWorkspaceServices workspaceServices)
=> new CompileTimeSolutionProvider(workspaceServices.Workspace);
}

private const string RazorEncConfigFileName = "RazorSourceGenerator.razorencconfig";

private readonly Workspace _workspace;
private readonly object _gate = new();

private Solution? _lazyCompileTimeSolution;
tmat marked this conversation as resolved.
Show resolved Hide resolved
private int? _correspondingDesignTimeSolutionVersion;
private readonly bool _enabled;

public CompileTimeSolutionProvider(Workspace workspace)
{
_workspace = workspace;
_enabled = workspace.Services.GetRequiredService<IExperimentationService>().IsExperimentEnabled(WellKnownExperimentNames.RazorLspEditorFeatureFlag);

workspace.WorkspaceChanged += (s, e) =>
{
if (e.Kind is WorkspaceChangeKind.SolutionCleared or WorkspaceChangeKind.SolutionRemoved)
{
lock (_gate)
{
_lazyCompileTimeSolution = null;
_correspondingDesignTimeSolutionVersion = null;
}
}
};
}

private static bool IsRazorAnalyzerConfig(TextDocumentState documentState)
=> documentState.FilePath != null && documentState.FilePath.EndsWith(RazorEncConfigFileName, StringComparison.OrdinalIgnoreCase);

public Solution GetCurrentCompileTimeSolution()
{
if (!_enabled)
{
return _workspace.CurrentSolution;
}

lock (_gate)
{
var currentDesignTimeSolution = _workspace.CurrentSolution;

// Design time solution hasn't changed since we calculated the last compile-time solution:
if (currentDesignTimeSolution.WorkspaceVersion == _correspondingDesignTimeSolutionVersion)
{
Contract.ThrowIfNull(_lazyCompileTimeSolution);
return _lazyCompileTimeSolution;
}

using var _1 = ArrayBuilder<DocumentId>.GetInstance(out var configIdsToRemove);
using var _2 = ArrayBuilder<DocumentId>.GetInstance(out var documentIdsToRemove);

var compileTimeSolution = currentDesignTimeSolution;

foreach (var (_, projectState) in currentDesignTimeSolution.State.ProjectStates)
{
var anyConfigs = false;

foreach (var configState in projectState.AnalyzerConfigDocumentStates.States)
{
if (IsRazorAnalyzerConfig(configState))
{
configIdsToRemove.Add(configState.Id);
anyConfigs = true;
}
}

// only remove design-time only documents when source-generated ones replace them
if (anyConfigs)
{
foreach (var documentState in projectState.DocumentStates.States)
{
if (documentState.Attributes.DesignTimeOnly)
{
documentIdsToRemove.Add(documentState.Id);
}
}
}
}

_lazyCompileTimeSolution = currentDesignTimeSolution
.RemoveAnalyzerConfigDocuments(configIdsToRemove.ToImmutable())
.RemoveDocuments(documentIdsToRemove.ToImmutable());
Comment on lines +116 to +118
Copy link
Member

Choose a reason for hiding this comment

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

Note that trying to do a "single" remove operation here rather than just putting it inside the loop over each project state might actually be slower: RemoveDocuments here only works by doing a GroupBy(d => d.ProjectId) and then processing each group individually.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I'll revisit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Observations:

  1. The equivalent Remove methods on Project call the Solution ones - so removing multiple documents from each project is worse than from the Solution.
  2. Removing each document individually seems to create a lot throw away objects.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll follow up on this in a separate PR if we conclude there is a better way.


_correspondingDesignTimeSolutionVersion = currentDesignTimeSolution.WorkspaceVersion;
return _lazyCompileTimeSolution;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// 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.

namespace Microsoft.CodeAnalysis.Host
{
/// <summary>
/// Provides a compile-time view of the current workspace solution.
/// Workaround for Razor projects which generate both design-time and compile-time source files.
/// TODO: remove https://github.com/dotnet/roslyn/issues/51678
/// </summary>
internal interface ICompileTimeSolutionProvider : IWorkspaceService
{
Solution GetCurrentCompileTimeSolution();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.EditAndContinue;
using Microsoft.CodeAnalysis.Editor.Implementation.EditAndContinue;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.VisualStudio.Debugger.Contracts.EditAndContinue;
Expand Down Expand Up @@ -52,6 +53,9 @@ public ManagedEditAndContinueLanguageService(
_diagnosticUpdateSource = diagnosticUpdateSource;
}

private Solution GetCurrentCompileTimeSolution()
=> _proxy.Workspace.Services.GetRequiredService<ICompileTimeSolutionProvider>().GetCurrentCompileTimeSolution();

/// <summary>
/// Called by the debugger when a debugging session starts and managed debugging is being used.
/// </summary>
Expand All @@ -67,7 +71,7 @@ public async Task StartDebuggingAsync(DebugSessionFlags flags, CancellationToken

try
{
var solution = _proxy.Workspace.CurrentSolution;
var solution = GetCurrentCompileTimeSolution();
_debuggingSessionConnection = await _proxy.StartDebuggingSessionAsync(solution, _debuggerService, captureMatchingDocuments: false, cancellationToken).ConfigureAwait(false);
}
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e, cancellationToken))
Expand All @@ -85,7 +89,7 @@ public async Task EnterBreakStateAsync(CancellationToken cancellationToken)
return;
}

var solution = _proxy.Workspace.CurrentSolution;
var solution = GetCurrentCompileTimeSolution();

try
{
Expand Down Expand Up @@ -175,7 +179,7 @@ public async Task<bool> HasChangesAsync(string? sourceFilePath, CancellationToke
{
try
{
var solution = _proxy.Workspace.CurrentSolution;
var solution = GetCurrentCompileTimeSolution();
var activeStatementSpanProvider = GetActiveStatementSpanProvider(solution);
return await _proxy.HasChangesAsync(solution, activeStatementSpanProvider, sourceFilePath, cancellationToken).ConfigureAwait(false);
}
Expand All @@ -189,7 +193,7 @@ public async Task<ManagedModuleUpdates> GetManagedModuleUpdatesAsync(Cancellatio
{
try
{
var solution = _proxy.Workspace.CurrentSolution;
var solution = GetCurrentCompileTimeSolution();
var activeStatementSpanProvider = GetActiveStatementSpanProvider(solution);
return await _proxy.EmitSolutionUpdateAsync(solution, activeStatementSpanProvider, _diagnosticService, _diagnosticUpdateSource, cancellationToken).ConfigureAwait(false);
}
Expand All @@ -203,7 +207,7 @@ public async Task<ManagedModuleUpdates> GetManagedModuleUpdatesAsync(Cancellatio
{
try
{
var solution = _proxy.Workspace.CurrentSolution;
var solution = GetCurrentCompileTimeSolution();

var activeStatementSpanProvider = new SolutionActiveStatementSpanProvider(async (documentId, cancellationToken) =>
{
Expand All @@ -224,7 +228,7 @@ public async Task<ManagedModuleUpdates> GetManagedModuleUpdatesAsync(Cancellatio
{
try
{
var solution = _proxy.Workspace.CurrentSolution;
var solution = GetCurrentCompileTimeSolution();
return await _proxy.IsActiveStatementInExceptionRegionAsync(solution, instruction, cancellationToken).ConfigureAwait(false);
}
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e, cancellationToken))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,6 @@ internal static class WellKnownExperimentNames
public const string RemoveUnusedReferences = "Roslyn.RemoveUnusedReferences";
public const string LSPCompletion = "Roslyn.LSP.Completion";
public const string UnnamedSymbolCompletionDisabled = "Roslyn.UnnamedSymbolCompletionDisabled";
public const string RazorLspEditorFeatureFlag = "Razor.LSP.Editor";
tmat marked this conversation as resolved.
Show resolved Hide resolved
}
}