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

Adds parallel execution of background Roslyn analyzers #2312

Closed
wants to merge 16 commits into from
Closed
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
@@ -0,0 +1,9 @@
namespace OmniSharp.Models.Events
{
public enum BackgroundDiagnosticStatus
{
Started = 0,
Progress = 1,
Finished = 2
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
namespace OmniSharp.Models.Events
{
public class BackgroundDiagnosticStatusMessage
{
public BackgroundDiagnosticStatus Status { get; set; }
public int NumberProjects { get; set; }
public int NumberFilesTotal { get; set; }
public int NumberFilesRemaining { get; set; }
}
}
4 changes: 2 additions & 2 deletions src/OmniSharp.Abstractions/Models/Events/EventTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public static class EventTypes
public const string PackageRestoreFinished = nameof(PackageRestoreFinished);
public const string UnresolvedDependencies = nameof(UnresolvedDependencies);
public const string ProjectConfiguration = nameof(ProjectConfiguration);
public const string ProjectDiagnosticStatus = nameof(ProjectDiagnosticStatus);

public const string ProjectDiagnosticStatus = nameof(ProjectDiagnosticStatus); // Obsolete, retained for compatibility with older clients
public const string BackgroundDiagnosticStatus = nameof(BackgroundDiagnosticStatus);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ public class ProjectDiagnosticStatusMessage
public string ProjectFilePath { get; set; }
public string Type = "background";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.Extensions.Logging;
using OmniSharp.Helpers;
using OmniSharp.Models.Diagnostics;
using OmniSharp.Options;
using OmniSharp.Roslyn.CSharp.Services.Diagnostics;

namespace OmniSharp.Roslyn.CSharp.Workers.Diagnostics
Expand All @@ -22,14 +23,16 @@ public class CSharpDiagnosticWorker: ICsDiagnosticWorker, IDisposable
private readonly ILogger _logger;
private readonly OmniSharpWorkspace _workspace;
private readonly DiagnosticEventForwarder _forwarder;
private readonly OmniSharpOptions _options;
private readonly IObserver<string> _openDocuments;
private readonly IDisposable _disposable;

public CSharpDiagnosticWorker(OmniSharpWorkspace workspace, DiagnosticEventForwarder forwarder, ILoggerFactory loggerFactory)
public CSharpDiagnosticWorker(OmniSharpWorkspace workspace, DiagnosticEventForwarder forwarder, ILoggerFactory loggerFactory, OmniSharpOptions options)
{
_workspace = workspace;
_forwarder = forwarder;
_logger = loggerFactory.CreateLogger<CSharpDiagnosticWorker>();
_options = options;

var openDocumentsSubject = new Subject<string>();
_openDocuments = openDocumentsSubject;
Expand Down Expand Up @@ -146,15 +149,32 @@ public async Task<ImmutableArray<DocumentDiagnostics>> GetDiagnostics(ImmutableA
.Select(docPath => _workspace.GetDocumentsFromFullProjectModelAsync(docPath)))
).SelectMany(s => s);

var diagnosticTasks = new List<Task>();
var throttler = new SemaphoreSlim(_options.RoslynExtensionsOptions.DiagnosticWorkersThreadCount);
foreach (var document in documents)
{
if(document?.Project?.Name == null)
continue;

var projectName = document.Project.Name;
var diagnostics = await GetDiagnosticsForDocument(document, projectName);
results.Add(new DocumentDiagnostics(document.Id, document.FilePath, document.Project.Id, document.Project.Name, diagnostics));
await throttler.WaitAsync();
diagnosticTasks.Add(
Task.Run(async () =>
{
try
{
var diagnostics = await GetDiagnosticsForDocument(document, projectName);
results.Add(new DocumentDiagnostics(document.Id, document.FilePath, document.Project.Id, document.Project.Name, diagnostics));
}
finally
{
throttler.Release();
}
}
)
);
}
await Task.WhenAll(diagnosticTasks);

return results.ToImmutableArray();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Options;
using Microsoft.Extensions.Logging;
using OmniSharp.Helpers;
using OmniSharp.Models.Diagnostics;
Expand All @@ -23,10 +22,10 @@ namespace OmniSharp.Roslyn.CSharp.Services.Diagnostics
public class CSharpDiagnosticWorkerWithAnalyzers : ICsDiagnosticWorker, IDisposable
{
private readonly AnalyzerWorkQueue _workQueue;
private readonly SemaphoreSlim _throttler;
private readonly ILogger<CSharpDiagnosticWorkerWithAnalyzers> _logger;

private readonly ConcurrentDictionary<DocumentId, DocumentDiagnostics> _currentDiagnosticResultLookup =
new ConcurrentDictionary<DocumentId, DocumentDiagnostics>();
private readonly ConcurrentDictionary<DocumentId, DocumentDiagnostics> _currentDiagnosticResultLookup = new();
private readonly ImmutableArray<ICodeActionProvider> _providers;
private readonly DiagnosticEventForwarder _forwarder;
private readonly OmniSharpOptions _options;
Expand All @@ -47,6 +46,7 @@ public CSharpDiagnosticWorkerWithAnalyzers(
_logger = loggerFactory.CreateLogger<CSharpDiagnosticWorkerWithAnalyzers>();
_providers = providers.ToImmutableArray();
_workQueue = new AnalyzerWorkQueue(loggerFactory, timeoutForPendingWorkMs: options.RoslynExtensionsOptions.DocumentAnalysisTimeoutMs * 3);
_throttler = new SemaphoreSlim(options.RoslynExtensionsOptions.DiagnosticWorkersThreadCount);

_forwarder = forwarder;
_options = options;
Expand Down Expand Up @@ -117,22 +117,51 @@ private async Task Worker(AnalyzerWorkType workType)
{
var solution = _workspace.CurrentSolution;

var currentWorkGroupedByProjects = _workQueue
var documents = _workQueue
.TakeWork(workType)
.Select(documentId => (projectId: solution.GetDocument(documentId)?.Project?.Id, documentId))
.Where(x => x.projectId != null)
.GroupBy(x => x.projectId, x => x.documentId)
.ToImmutableArray();

foreach (var projectGroup in currentWorkGroupedByProjects)
{
var projectPath = solution.GetProject(projectGroup.Key).FilePath;
var documentCount = documents.Length;
var documentCountRemaining = documentCount;

// event every percentage increase, or every 10th if there are fewer than 1000
var eventEvery = Math.Max(10, documentCount / 100);

EventIfBackgroundWork(workType, projectPath, ProjectDiagnosticStatus.Started);
var documentsGroupedByProjects = documents
.GroupBy(x => x.projectId, x => x.documentId)
.ToImmutableArray();
var projectCount = documentsGroupedByProjects.Length;

await AnalyzeProject(solution, projectGroup);
EventIfBackgroundWork(workType, BackgroundDiagnosticStatus.Started, projectCount, documentCount, documentCountRemaining);

EventIfBackgroundWork(workType, projectPath, ProjectDiagnosticStatus.Ready);
void decrementDocumentCountRemaining()
{
var remaining = Interlocked.Decrement(ref documentCountRemaining);
var done = documentCount - remaining;
if (done % eventEvery == 0)
{
EventIfBackgroundWork(workType, BackgroundDiagnosticStatus.Progress, projectCount, documentCount, remaining);
}
}

try
{
var projectAnalyzerTasks =
documentsGroupedByProjects
.Select(projectGroup => Task.Run(async () =>
{
var projectPath = solution.GetProject(projectGroup.Key).FilePath;
await AnalyzeProject(solution, projectGroup, decrementDocumentCountRemaining);
}))
.ToImmutableArray();

await Task.WhenAll(projectAnalyzerTasks);
}
finally
{
EventIfBackgroundWork(workType, BackgroundDiagnosticStatus.Finished, projectCount, documentCount, documentCountRemaining);
}

_workQueue.WorkComplete(workType);
Expand All @@ -146,10 +175,10 @@ private async Task Worker(AnalyzerWorkType workType)
}
}

private void EventIfBackgroundWork(AnalyzerWorkType workType, string projectPath, ProjectDiagnosticStatus status)
private void EventIfBackgroundWork(AnalyzerWorkType workType, BackgroundDiagnosticStatus status, int numberProjects, int numberFiles, int numberFilesRemaining)
{
if (workType == AnalyzerWorkType.Background)
_forwarder.ProjectAnalyzedInBackground(projectPath, status);
_forwarder.BackgroundDiagnosticsStatus(status, numberProjects, numberFiles, numberFilesRemaining);
}

private void QueueForAnalysis(ImmutableArray<DocumentId> documentIds, AnalyzerWorkType workType)
Expand Down Expand Up @@ -188,7 +217,6 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEv
case WorkspaceChangeKind.SolutionReloaded:
QueueDocumentsForDiagnostics();
break;

}
}

Expand All @@ -215,8 +243,9 @@ public async Task<IEnumerable<Diagnostic>> AnalyzeProjectsAsync(Project project,
.Concat(project.AnalyzerReferences.SelectMany(x => x.GetAnalyzers(project.Language)))
.ToImmutableArray();

var compilation = await project.GetCompilationAsync();
var compilation = await project.GetCompilationAsync(cancellationToken);
var workspaceAnalyzerOptions = (AnalyzerOptions)_workspaceAnalyzerOptionsConstructor.Invoke(new object[] { project.AnalyzerOptions, project.Solution });
var analyzerTasks = new List<Task>();

foreach (var document in project.Documents)
{
Expand All @@ -227,7 +256,7 @@ public async Task<IEnumerable<Diagnostic>> AnalyzeProjectsAsync(Project project,
return diagnostics;
}

private async Task AnalyzeProject(Solution solution, IGrouping<ProjectId, DocumentId> documentsGroupedByProject)
private async Task AnalyzeProject(Solution solution, IGrouping<ProjectId, DocumentId> documentsGroupedByProject, Action decrementRemaining)
{
try
{
Expand All @@ -239,15 +268,30 @@ private async Task AnalyzeProject(Solution solution, IGrouping<ProjectId, Docume
.ToImmutableArray();

var compilation = await project.GetCompilationAsync();

var workspaceAnalyzerOptions = (AnalyzerOptions)_workspaceAnalyzerOptionsConstructor.Invoke(new object[] { project.AnalyzerOptions, project.Solution });
var documentAnalyzerTasks = new List<Task>();

foreach (var documentId in documentsGroupedByProject)
{
var document = project.GetDocument(documentId);
var diagnostics = await AnalyzeDocument(project, allAnalyzers, compilation, workspaceAnalyzerOptions, document);
UpdateCurrentDiagnostics(project, document, diagnostics);
await _throttler.WaitAsync();

documentAnalyzerTasks.Add(Task.Run(async () =>
{
try
{
var document = project.GetDocument(documentId);
var diagnostics = await AnalyzeDocument(project, allAnalyzers, compilation, workspaceAnalyzerOptions, document);
UpdateCurrentDiagnostics(project, document, diagnostics);
decrementRemaining();
}
finally
{
_throttler.Release();
}
}));
}

await Task.WhenAll(documentAnalyzerTasks);
}
catch (Exception ex)
{
Expand All @@ -265,14 +309,12 @@ private async Task<ImmutableArray<Diagnostic>> AnalyzeDocument(Project project,

var documentSemanticModel = await document.GetSemanticModelAsync(perDocumentTimeout.Token);

var diagnostics = ImmutableArray<Diagnostic>.Empty;

// Only basic syntax check is available if file is miscellanous like orphan .cs file.
// Those projects are on hard coded virtual project
if (project.Name == $"{Configuration.OmniSharpMiscProjectName}.csproj")
{
var syntaxTree = await document.GetSyntaxTreeAsync();
diagnostics = syntaxTree.GetDiagnostics().ToImmutableArray();
return syntaxTree.GetDiagnostics().ToImmutableArray();
}
else if (allAnalyzers.Any()) // Analyzers cannot be called with empty analyzer list.
{
Expand All @@ -289,18 +331,16 @@ private async Task<ImmutableArray<Diagnostic>> AnalyzeDocument(Project project,
var syntaxDiagnosticsWithAnalyzers = await compilationWithAnalyzers
.GetAnalyzerSyntaxDiagnosticsAsync(documentSemanticModel.SyntaxTree, perDocumentTimeout.Token);

diagnostics = semanticDiagnosticsWithAnalyzers
return semanticDiagnosticsWithAnalyzers
.Concat(syntaxDiagnosticsWithAnalyzers)
.Where(d => !d.IsSuppressed)
.Concat(documentSemanticModel.GetDiagnostics())
.ToImmutableArray();
}
else
{
diagnostics = documentSemanticModel.GetDiagnostics();
return documentSemanticModel.GetDiagnostics();
}

return diagnostics;
}
catch (Exception ex)
{
Expand All @@ -311,7 +351,7 @@ private async Task<ImmutableArray<Diagnostic>> AnalyzeDocument(Project project,

private void OnAnalyzerException(Exception ex, DiagnosticAnalyzer analyzer, Diagnostic diagnostic)
{
_logger.LogDebug($"Exception in diagnostic analyzer." +
_logger.LogDebug("Exception in diagnostic analyzer." +
$"\n analyzer: {analyzer}" +
$"\n diagnostic: {diagnostic}" +
$"\n exception: {ex.Message}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class CsharpDiagnosticWorkerComposer: ICsDiagnosticWorker, IDisposable
private readonly IEnumerable<ICodeActionProvider> _providers;
private readonly ILoggerFactory _loggerFactory;
private readonly DiagnosticEventForwarder _forwarder;
private readonly IOptionsMonitor<OmniSharpOptions> _options;
private ICsDiagnosticWorker _implementation;
private readonly IDisposable _onChange;

Expand All @@ -37,6 +38,7 @@ public CsharpDiagnosticWorkerComposer(
_providers = providers;
_loggerFactory = loggerFactory;
_forwarder = forwarder;
_options = options;
_onChange = options.OnChange(UpdateImplementation);
UpdateImplementation(options.CurrentValue);
}
Expand All @@ -54,7 +56,7 @@ private void UpdateImplementation(OmniSharpOptions options)
}
else if (!options.RoslynExtensionsOptions.EnableAnalyzersSupport && (firstRun || _implementation is CSharpDiagnosticWorkerWithAnalyzers))
{
var old = Interlocked.Exchange(ref _implementation, new CSharpDiagnosticWorker(_workspace, _forwarder, _loggerFactory));
var old = Interlocked.Exchange(ref _implementation, new CSharpDiagnosticWorker(_workspace, _forwarder, _loggerFactory, _options.CurrentValue));
if (old is IDisposable disposable)
{
disposable.Dispose();
Expand Down
32 changes: 30 additions & 2 deletions src/OmniSharp.Roslyn/DiagnosticEventForwarder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,37 @@ public void Forward(DiagnosticMessage message)
_emitter.Emit(EventTypes.Diagnostic, message);
}

public void ProjectAnalyzedInBackground(string projectFileName, ProjectDiagnosticStatus status)
public void BackgroundDiagnosticsStatus(BackgroundDiagnosticStatus status, int numberProjects, int numberFiles, int numberFilesRemaining)
{
_emitter.Emit(EventTypes.ProjectDiagnosticStatus, new ProjectDiagnosticStatusMessage { ProjectFilePath = projectFileName, Status = status });
// New type of background diagnostic event, allows more control of visualization in clients:
_emitter.Emit(EventTypes.BackgroundDiagnosticStatus, new BackgroundDiagnosticStatusMessage
{
Status = status,
NumberProjects = numberProjects,
NumberFilesTotal = numberFiles,
NumberFilesRemaining = numberFilesRemaining
});

// Old type of event emitted as a shim for older clients:
double percentComplete = 0;
if (numberFiles > 0 && numberFiles > numberFilesRemaining)
{
percentComplete = numberFiles <= 0
? 100
: (numberFiles - numberFilesRemaining) / (double)numberFiles;
}

_emitter.Emit(EventTypes.ProjectDiagnosticStatus, new ProjectDiagnosticStatusMessage
{
// There is no current project file being analyzed anymore since all the analysis
// executes concurrently, but we have to supply some value for the ProjectFilePath
// property for clients that only know about this event. In VS Code the following
// displays nicely as "Analyzing (24%)".
ProjectFilePath = $"({percentComplete:P0})",
Status = status == BackgroundDiagnosticStatus.Finished ?
ProjectDiagnosticStatus.Ready :
ProjectDiagnosticStatus.Started
});
}
}
}
3 changes: 2 additions & 1 deletion src/OmniSharp.Shared/Options/RoslynExtensionsOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public class RoslynExtensionsOptions : OmniSharpExtensionsOptions
public bool EnableAnalyzersSupport { get; set; }
public bool EnableImportCompletion { get; set; }
public bool EnableAsyncCompletion { get; set; }
public int DocumentAnalysisTimeoutMs { get; set; } = 10 * 1000;
public int DocumentAnalysisTimeoutMs { get; set; } = 30 * 1000;
public int DiagnosticWorkersThreadCount { get; set; } = Math.Max(1, (int)(Environment.ProcessorCount * 0.75)); // Use 75% of available processors by default (but at least one)
}

public class OmniSharpExtensionsOptions
Expand Down
Loading