diff --git a/src/OmniSharp.Abstractions/Models/Events/BackgroundDiagnosticStatus.cs b/src/OmniSharp.Abstractions/Models/Events/BackgroundDiagnosticStatus.cs new file mode 100644 index 0000000000..8b5e180618 --- /dev/null +++ b/src/OmniSharp.Abstractions/Models/Events/BackgroundDiagnosticStatus.cs @@ -0,0 +1,9 @@ +namespace OmniSharp.Models.Events +{ + public enum BackgroundDiagnosticStatus + { + Started = 0, + Progress = 1, + Finished = 2 + } +} diff --git a/src/OmniSharp.Abstractions/Models/Events/BackgroundDiagnosticStatusMessage.cs b/src/OmniSharp.Abstractions/Models/Events/BackgroundDiagnosticStatusMessage.cs new file mode 100644 index 0000000000..40fe2b6927 --- /dev/null +++ b/src/OmniSharp.Abstractions/Models/Events/BackgroundDiagnosticStatusMessage.cs @@ -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; } + } +} diff --git a/src/OmniSharp.Abstractions/Models/Events/EventTypes.cs b/src/OmniSharp.Abstractions/Models/Events/EventTypes.cs index 38da098cbb..021439eade 100644 --- a/src/OmniSharp.Abstractions/Models/Events/EventTypes.cs +++ b/src/OmniSharp.Abstractions/Models/Events/EventTypes.cs @@ -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); } } diff --git a/src/OmniSharp.Abstractions/Models/Events/ProjectAnalyzeStatusMessage.cs b/src/OmniSharp.Abstractions/Models/Events/ProjectDiagnosticStatusMessage.cs similarity index 99% rename from src/OmniSharp.Abstractions/Models/Events/ProjectAnalyzeStatusMessage.cs rename to src/OmniSharp.Abstractions/Models/Events/ProjectDiagnosticStatusMessage.cs index bd84bf6cd9..c1e47a7153 100644 --- a/src/OmniSharp.Abstractions/Models/Events/ProjectAnalyzeStatusMessage.cs +++ b/src/OmniSharp.Abstractions/Models/Events/ProjectDiagnosticStatusMessage.cs @@ -6,4 +6,4 @@ public class ProjectDiagnosticStatusMessage public string ProjectFilePath { get; set; } public string Type = "background"; } -} \ No newline at end of file +} diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs index 1299720e24..56f6e9fa88 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs @@ -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 @@ -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 _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(); + _options = options; var openDocumentsSubject = new Subject(); _openDocuments = openDocumentsSubject; @@ -146,15 +149,32 @@ public async Task> GetDiagnostics(ImmutableA .Select(docPath => _workspace.GetDocumentsFromFullProjectModelAsync(docPath))) ).SelectMany(s => s); + var diagnosticTasks = new List(); + 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(); } diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs index 31ec749afd..a9597d0ec1 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs @@ -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; @@ -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 _logger; - private readonly ConcurrentDictionary _currentDiagnosticResultLookup = - new ConcurrentDictionary(); + private readonly ConcurrentDictionary _currentDiagnosticResultLookup = new(); private readonly ImmutableArray _providers; private readonly DiagnosticEventForwarder _forwarder; private readonly OmniSharpOptions _options; @@ -47,6 +46,7 @@ public CSharpDiagnosticWorkerWithAnalyzers( _logger = loggerFactory.CreateLogger(); _providers = providers.ToImmutableArray(); _workQueue = new AnalyzerWorkQueue(loggerFactory, timeoutForPendingWorkMs: options.RoslynExtensionsOptions.DocumentAnalysisTimeoutMs * 3); + _throttler = new SemaphoreSlim(options.RoslynExtensionsOptions.DiagnosticWorkersThreadCount); _forwarder = forwarder; _options = options; @@ -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); @@ -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 documentIds, AnalyzerWorkType workType) @@ -188,7 +217,6 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEv case WorkspaceChangeKind.SolutionReloaded: QueueDocumentsForDiagnostics(); break; - } } @@ -215,8 +243,9 @@ public async Task> 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(); foreach (var document in project.Documents) { @@ -227,7 +256,7 @@ public async Task> AnalyzeProjectsAsync(Project project, return diagnostics; } - private async Task AnalyzeProject(Solution solution, IGrouping documentsGroupedByProject) + private async Task AnalyzeProject(Solution solution, IGrouping documentsGroupedByProject, Action decrementRemaining) { try { @@ -239,15 +268,30 @@ private async Task AnalyzeProject(Solution solution, IGrouping(); 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) { @@ -265,14 +309,12 @@ private async Task> AnalyzeDocument(Project project, var documentSemanticModel = await document.GetSemanticModelAsync(perDocumentTimeout.Token); - var diagnostics = ImmutableArray.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. { @@ -289,7 +331,7 @@ private async Task> 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()) @@ -297,10 +339,8 @@ private async Task> AnalyzeDocument(Project project, } else { - diagnostics = documentSemanticModel.GetDiagnostics(); + return documentSemanticModel.GetDiagnostics(); } - - return diagnostics; } catch (Exception ex) { @@ -311,7 +351,7 @@ private async Task> 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}"); diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs index 6b95507530..beefa5515a 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs @@ -22,6 +22,7 @@ public class CsharpDiagnosticWorkerComposer: ICsDiagnosticWorker, IDisposable private readonly IEnumerable _providers; private readonly ILoggerFactory _loggerFactory; private readonly DiagnosticEventForwarder _forwarder; + private readonly IOptionsMonitor _options; private ICsDiagnosticWorker _implementation; private readonly IDisposable _onChange; @@ -37,6 +38,7 @@ public CsharpDiagnosticWorkerComposer( _providers = providers; _loggerFactory = loggerFactory; _forwarder = forwarder; + _options = options; _onChange = options.OnChange(UpdateImplementation); UpdateImplementation(options.CurrentValue); } @@ -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(); diff --git a/src/OmniSharp.Roslyn/DiagnosticEventForwarder.cs b/src/OmniSharp.Roslyn/DiagnosticEventForwarder.cs index f4f3f090cd..23cd93566d 100644 --- a/src/OmniSharp.Roslyn/DiagnosticEventForwarder.cs +++ b/src/OmniSharp.Roslyn/DiagnosticEventForwarder.cs @@ -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 + }); } } } diff --git a/src/OmniSharp.Shared/Options/RoslynExtensionsOptions.cs b/src/OmniSharp.Shared/Options/RoslynExtensionsOptions.cs index a35e49bd02..9744c833b9 100644 --- a/src/OmniSharp.Shared/Options/RoslynExtensionsOptions.cs +++ b/src/OmniSharp.Shared/Options/RoslynExtensionsOptions.cs @@ -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 diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FixAllFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FixAllFacts.cs index 545132bed9..81a26e43e6 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FixAllFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FixAllFacts.cs @@ -15,12 +15,12 @@ namespace OmniSharp.Roslyn.CSharp.Tests public class FixAllFacts { private readonly ITestOutputHelper _testOutput; - private readonly TestEventEmitter _analysisEventListener; + private readonly TestEventEmitter _analysisEventListener; public FixAllFacts(ITestOutputHelper testOutput) { _testOutput = testOutput; - _analysisEventListener = new TestEventEmitter(); + _analysisEventListener = new TestEventEmitter(); } [Fact] diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/ReAnalysisFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/ReAnalysisFacts.cs index f302430cb0..d05663fc68 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/ReAnalysisFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/ReAnalysisFacts.cs @@ -15,12 +15,12 @@ namespace OmniSharp.Roslyn.CSharp.Tests public class ReAnalysisFacts { private readonly ITestOutputHelper _testOutput; - private readonly TestEventEmitter _eventListener; + private readonly TestEventEmitter _eventListener; public ReAnalysisFacts(ITestOutputHelper testOutput) { _testOutput = testOutput; - _eventListener = new TestEventEmitter(); + _eventListener = new TestEventEmitter(); } @@ -74,8 +74,8 @@ public async Task WhenReanalyzeIsExecuted_ThenSendEventWhenAnalysisOfProjectIsRe await reAnalyzeHandler.Handle(new ReAnalyzeRequest()); - await _eventListener.ExpectForEmitted(x => x.ProjectFilePath == project.FilePath && x.Status == ProjectDiagnosticStatus.Started); - await _eventListener.ExpectForEmitted(x => x.ProjectFilePath == project.FilePath && x.Status == ProjectDiagnosticStatus.Ready); + await _eventListener.ExpectForEmitted(x => x.NumberFilesTotal == 1 && x.Status == BackgroundDiagnosticStatus.Started); + await _eventListener.ExpectForEmitted(x => x.NumberFilesTotal == 1 && x.Status == BackgroundDiagnosticStatus.Finished); } } @@ -96,8 +96,8 @@ await reAnalyzeHandler.Handle(new ReAnalyzeRequest FileName = projectA.Documents.Single(x => x.FilePath.EndsWith("a.cs")).FilePath }); - await _eventListener.ExpectForEmitted(x => x.ProjectFilePath == projectA.FilePath && x.Status == ProjectDiagnosticStatus.Started); - await _eventListener.ExpectForEmitted(x => x.ProjectFilePath == projectA.FilePath && x.Status == ProjectDiagnosticStatus.Ready); + await _eventListener.ExpectForEmitted(x => x.NumberFilesTotal == 1 && x.Status == BackgroundDiagnosticStatus.Started); + await _eventListener.ExpectForEmitted(x => x.NumberFilesTotal == 1 && x.Status == BackgroundDiagnosticStatus.Finished); } } @@ -118,8 +118,8 @@ await reAnalyzeHandler.Handle(new ReAnalyzeRequest FileName = project.FilePath }); - await _eventListener.ExpectForEmitted(x => x.ProjectFilePath == project.FilePath && x.Status == ProjectDiagnosticStatus.Started); - await _eventListener.ExpectForEmitted(x => x.ProjectFilePath == project.FilePath && x.Status == ProjectDiagnosticStatus.Ready); + await _eventListener.ExpectForEmitted(x => x.NumberFilesTotal == 1 && x.Status == BackgroundDiagnosticStatus.Started); + await _eventListener.ExpectForEmitted(x => x.NumberFilesTotal == 1 && x.Status == BackgroundDiagnosticStatus.Finished); } }