Merge pull request #1984 from 333fred/revert-caching-changes

Targeted DiagnosticWorker Revert
OmniSharp · Oct 14, 2020 · 26c0916 · 26c0916
2 parents 4046da8 + da75c1b
commit 26c0916
Show file tree

Hide file tree

Showing 13 changed files with 261 additions and 253 deletions.
diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/DiagnosticExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/DiagnosticExtensions.cs
@@ -35,7 +35,7 @@ internal static DiagnosticLocation ToDiagnosticLocation(this Diagnostic diagnost
         internal static IEnumerable<DiagnosticLocation> DistinctDiagnosticLocationsByProject(this IEnumerable<DocumentDiagnostics> documentDiagnostic)
         {
             return documentDiagnostic
-                .SelectMany(x => x.Diagnostics, (parent, child) => (projectName: parent.Project.Name, diagnostic: child))
+                .SelectMany(x => x.Diagnostics, (parent, child) => (projectName: parent.ProjectName, diagnostic: child))
                 .Select(x => new
                 {
                     location = x.diagnostic.ToDiagnosticLocation(),

diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Diagnostics/CodeCheckService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Diagnostics/CodeCheckService.cs
@@ -38,7 +38,7 @@ public async Task<QuickFixResponse> Handle(CodeCheckRequest request)
                 return GetResponseFromDiagnostics(allDiagnostics, fileName: null);
             }
 
-            var diagnostics = await _diagWorker.GetDiagnostics(new [] { request.FileName }.ToImmutableArray());
+            var diagnostics = await _diagWorker.GetDiagnostics(ImmutableArray.Create(request.FileName));
 
             return GetResponseFromDiagnostics(diagnostics, request.FileName);
         }
@@ -47,7 +47,7 @@ private static QuickFixResponse GetResponseFromDiagnostics(ImmutableArray<Docume
         {
             var diagnosticLocations = diagnostics
                 .Where(x => string.IsNullOrEmpty(fileName)
-                    || x.Document.FilePath == fileName)
+                    || x.DocumentPath == fileName)
                 .DistinctDiagnosticLocationsByProject()
                 .Where(x => x.FileName != null);
 

diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs
@@ -39,7 +39,7 @@ public override async Task<GetFixAllResponse> Handle(GetFixAllRequest request)
 
             var allDiagnostics = await GetDiagnosticsAsync(request.Scope, document);
             var validFixes = allDiagnostics
-                .GroupBy(docAndDiag => docAndDiag.Document.Project)
+                .GroupBy(docAndDiag => docAndDiag.ProjectId)
                 .SelectMany(grouping =>
                 {
                     var projectFixProviders = GetCodeFixProviders(grouping.Key);

diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs
@@ -192,26 +192,13 @@ public FixAllDiagnosticProvider(ICsDiagnosticWorker diagnosticWorker)
             }
 
             public override async Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Project project, CancellationToken cancellationToken)
-            {
-                var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.ToImmutableArray());
-                return diagnostics.SelectMany(x => x.Diagnostics);
-            }
+                => await _diagnosticWorker.AnalyzeProjectsAsync(project, cancellationToken);
 
             public override async Task<IEnumerable<Diagnostic>> GetDocumentDiagnosticsAsync(Document document, CancellationToken cancellationToken)
-            {
-                var documentDiagnostics = await _diagnosticWorker.GetDiagnostics(ImmutableArray.Create(document));
-
-                if (!documentDiagnostics.Any())
-                    return new Diagnostic[] { };
-
-                return documentDiagnostics.First().Diagnostics;
-            }
+                => await _diagnosticWorker.AnalyzeDocumentAsync(document, cancellationToken);
 
             public override async Task<IEnumerable<Diagnostic>> GetProjectDiagnosticsAsync(Project project, CancellationToken cancellationToken)
-            {
-                var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.ToImmutableArray());
-                return diagnostics.SelectMany(x => x.Diagnostics);
-            }
+                => await _diagnosticWorker.AnalyzeProjectsAsync(project, cancellationToken);
         }
     }
 }
diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs
@@ -127,7 +127,7 @@ private TextSpan GetTextSpan(ICodeActionRequest request, SourceText sourceText)
 
         private async Task CollectCodeFixesActions(Document document, TextSpan span, List<CodeAction> codeActions)
         {
-            var diagnosticsWithProjects = await _diagnostics.GetDiagnostics(ImmutableArray.Create(document));
+            var diagnosticsWithProjects = await _diagnostics.GetDiagnostics(ImmutableArray.Create(document.FilePath));
 
             var groupedBySpan = diagnosticsWithProjects
                     .SelectMany(x => x.Diagnostics)
@@ -167,7 +167,7 @@ private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerabl
 
         private List<CodeFixProvider> GetSortedCodeFixProviders(Document document)
         {
-            return ExtensionOrderer.GetOrderedOrUnorderedList<CodeFixProvider, ExportCodeFixProviderAttribute>(_codeFixesForProject.GetAllCodeFixesForProject(document.Project), attribute => attribute.Name).ToList();
+            return ExtensionOrderer.GetOrderedOrUnorderedList<CodeFixProvider, ExportCodeFixProviderAttribute>(_codeFixesForProject.GetAllCodeFixesForProject(document.Project.Id), attribute => attribute.Name).ToList();
         }
 
         private List<CodeRefactoringProvider> GetSortedCodeRefactoringProviders()
@@ -223,14 +223,14 @@ private IEnumerable<AvailableCodeAction> ConvertToAvailableCodeAction(IEnumerabl
             {
                 if (documentAndDiagnostics.Diagnostics.FirstOrDefault(d => d.Id == diagnosticId) is Diagnostic diagnostic)
                 {
-                    return (documentAndDiagnostics.Document.Id, diagnostic);
+                    return (documentAndDiagnostics.DocumentId, diagnostic);
                 }
             }
 
             return default;
         }
 
-        protected ImmutableArray<CodeFixProvider> GetCodeFixProviders(Project project)
+        protected ImmutableArray<CodeFixProvider> GetCodeFixProviders(ProjectId project)
         {
             return _codeFixesForProject.GetAllCodeFixesForProject(project);
         }
@@ -239,21 +239,21 @@ protected ImmutableArray<CodeFixProvider> GetCodeFixProviders(Project project)
         {
             // If Roslyn ever comes up with a UI for selecting what provider the user prefers, we might consider replicating.
             // https://github.com/dotnet/roslyn/issues/27066
-            return _codeFixesForProject.GetAllCodeFixesForProject(document.Project).FirstOrDefault(provider => provider.HasFixForId(id));
+            return _codeFixesForProject.GetAllCodeFixesForProject(document.Project.Id).FirstOrDefault(provider => provider.HasFixForId(id));
         }
 
         protected async Task<ImmutableArray<DocumentDiagnostics>> GetDiagnosticsAsync(FixAllScope scope, Document document)
         {
             switch (scope)
             {
                 case FixAllScope.Solution:
-                    var documentsInSolution = document.Project.Solution.Projects.SelectMany(p => p.Documents).ToImmutableArray();
+                    var documentsInSolution = document.Project.Solution.Projects.SelectMany(p => p.Documents).Select(d => d.FilePath).ToImmutableArray();
                     return await _diagnostics.GetDiagnostics(documentsInSolution);
                 case FixAllScope.Project:
-                    var documentsInProject = document.Project.Documents.ToImmutableArray();
+                    var documentsInProject = document.Project.Documents.Select(d => d.FilePath).ToImmutableArray();
                     return await _diagnostics.GetDiagnostics(documentsInProject);
                 case FixAllScope.Document:
-                    return await _diagnostics.GetDiagnostics(ImmutableArray.Create(document));
+                    return await _diagnostics.GetDiagnostics(ImmutableArray.Create(document.FilePath));
                 default:
                     throw new InvalidOperationException();
             }

diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs
@@ -41,14 +41,16 @@ public CachingCodeFixProviderForProjects(ILoggerFactory loggerFactory, OmniSharp
             };
         }
 
-        public ImmutableArray<CodeFixProvider> GetAllCodeFixesForProject(Project project)
+        public ImmutableArray<CodeFixProvider> GetAllCodeFixesForProject(ProjectId projectId)
         {
-            if (_cache.ContainsKey(project.Id))
-                return _cache[project.Id];
+            if (_cache.ContainsKey(projectId))
+                return _cache[projectId];
+
+            var project = _workspace.CurrentSolution.GetProject(projectId);
 
             if (project == null)
             {
-                _cache.TryRemove(project.Id, out _);
+                _cache.TryRemove(projectId, out _);
                 return ImmutableArray<CodeFixProvider>.Empty;
             }
 

diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/AnalyzerWorkQueue.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/AnalyzerWorkQueue.cs
@@ -17,8 +17,8 @@ public Queue(TimeSpan throttling)
                 Throttling = throttling;
             }
 
-            public ImmutableHashSet<Document> WorkWaitingToExecute { get; set; } = ImmutableHashSet<Document>.Empty;
-            public ImmutableHashSet<Document> WorkExecuting { get; set; } = ImmutableHashSet<Document>.Empty;
+            public ImmutableHashSet<DocumentId> WorkWaitingToExecute { get; set; } = ImmutableHashSet<DocumentId>.Empty;
+            public ImmutableHashSet<DocumentId> WorkExecuting { get; set; } = ImmutableHashSet<DocumentId>.Empty;
             public DateTime LastThrottlingBegan { get; set; } = DateTime.UtcNow;
             public TimeSpan Throttling { get; }
             public CancellationTokenSource WorkPendingToken { get; set; }
@@ -44,7 +44,7 @@ public AnalyzerWorkQueue(ILoggerFactory loggerFactory, int timeoutForPendingWork
             _maximumDelayWhenWaitingForResults = timeoutForPendingWorkMs;
         }
 
-        public void PutWork(IReadOnlyCollection<Document> documents, AnalyzerWorkType workType)
+        public void PutWork(IReadOnlyCollection<DocumentId> documentIds, AnalyzerWorkType workType)
         {
             lock (_queueLock)
             {
@@ -56,21 +56,21 @@ public void PutWork(IReadOnlyCollection<Document> documents, AnalyzerWorkType wo
                 if (queue.WorkPendingToken == null)
                     queue.WorkPendingToken = new CancellationTokenSource();
 
-                queue.WorkWaitingToExecute = queue.WorkWaitingToExecute.Union(documents);
+                queue.WorkWaitingToExecute = queue.WorkWaitingToExecute.Union(documentIds);
             }
         }
 
-        public IReadOnlyCollection<Document> TakeWork(AnalyzerWorkType workType)
+        public IReadOnlyCollection<DocumentId> TakeWork(AnalyzerWorkType workType)
         {
             lock (_queueLock)
             {
                 var queue = _queues[workType];
 
                 if (IsThrottlingActive(queue) || queue.WorkWaitingToExecute.IsEmpty)
-                    return ImmutableHashSet<Document>.Empty;
+                    return ImmutableHashSet<DocumentId>.Empty;
 
                 queue.WorkExecuting = queue.WorkWaitingToExecute;
-                queue.WorkWaitingToExecute = ImmutableHashSet<Document>.Empty;
+                queue.WorkWaitingToExecute = ImmutableHashSet<DocumentId>.Empty;
                 return queue.WorkExecuting;
             }
         }
@@ -84,12 +84,12 @@ public void WorkComplete(AnalyzerWorkType workType)
         {
             lock (_queueLock)
             {
-                if (_queues[workType].WorkExecuting.IsEmpty)
+                if(_queues[workType].WorkExecuting.IsEmpty)
                     return;
 
                 _queues[workType].WorkPendingToken?.Cancel();
                 _queues[workType].WorkPendingToken = null;
-                _queues[workType].WorkExecuting = ImmutableHashSet<Document>.Empty;
+                _queues[workType].WorkExecuting = ImmutableHashSet<DocumentId>.Empty;
             }
         }
 
@@ -107,9 +107,15 @@ public Task WaitForegroundWorkComplete()
                 .ContinueWith(task => LogTimeouts(task));
         }
 
-        public void QueueDocumentForeground(Document document)
+        public bool TryPromote(DocumentId id)
         {
-            PutWork(new[] { document }, AnalyzerWorkType.Foreground);
+            if (_queues[AnalyzerWorkType.Background].WorkWaitingToExecute.Contains(id) || _queues[AnalyzerWorkType.Background].WorkExecuting.Contains(id))
+            {
+                PutWork(new[] { id }, AnalyzerWorkType.Foreground);
+                return true;
+            }
+
+            return false;
         }
 
         private void LogTimeouts(Task task)

diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs
@@ -7,6 +7,7 @@
 using System.Reactive.Concurrency;
 using System.Reactive.Linq;
 using System.Reactive.Subjects;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
 using Microsoft.Extensions.Logging;
@@ -145,25 +146,14 @@ public async Task<ImmutableArray<DocumentDiagnostics>> GetDiagnostics(ImmutableA
                         .Select(docPath => _workspace.GetDocumentsFromFullProjectModelAsync(docPath)))
                 ).SelectMany(s => s);
 
-            return await GetDiagnostics(documents);
-        }
-
-        public Task<ImmutableArray<DocumentDiagnostics>> GetDiagnostics(ImmutableArray<Document> documents)
-        {
-            return GetDiagnostics((IEnumerable<Document>)documents);
-        }
-
-        private async Task<ImmutableArray<DocumentDiagnostics>> GetDiagnostics(IEnumerable<Document> documents)
-        {
-            var results = new List<DocumentDiagnostics>();
             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, diagnostics));
+                results.Add(new DocumentDiagnostics(document.Id, document.FilePath, document.Project.Id, document.Project.Name, diagnostics));
             }
 
             return results.ToImmutableArray();
@@ -185,18 +175,18 @@ private static async Task<ImmutableArray<Diagnostic>> GetDiagnosticsForDocument(
             }
         }
 
-        public ImmutableArray<Document> QueueDocumentsForDiagnostics()
+        public ImmutableArray<DocumentId> QueueDocumentsForDiagnostics()
         {
-            var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).ToImmutableArray();
+            var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents);
             QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray());
-            return documents;
+            return documents.Select(x => x.Id).ToImmutableArray();
         }
 
-        public ImmutableArray<Document> QueueDocumentsForDiagnostics(ImmutableArray<ProjectId> projectIds)
+        public ImmutableArray<DocumentId> QueueDocumentsForDiagnostics(ImmutableArray<ProjectId> projectIds)
         {
-            var documents = projectIds.SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents).ToImmutableArray();
+            var documents = projectIds.SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents);
             QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray());
-            return documents;
+            return documents.Select(x => x.Id).ToImmutableArray();
         }
 
         public Task<ImmutableArray<DocumentDiagnostics>> GetAllDiagnosticsAsync()
@@ -212,5 +202,23 @@ public void Dispose()
             _workspace.DocumentClosed -= OnDocumentOpened;
             _disposable.Dispose();
         }
+
+        public async Task<IEnumerable<Diagnostic>> AnalyzeDocumentAsync(Document document, CancellationToken cancellationToken)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            return await GetDiagnosticsForDocument(document, document.Project.Name);
+        }
+
+        public async Task<IEnumerable<Diagnostic>> AnalyzeProjectsAsync(Project project, CancellationToken cancellationToken)
+        {
+            var diagnostics = new List<Diagnostic>();
+            foreach (var document in project.Documents)
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+                diagnostics.AddRange(await GetDiagnosticsForDocument(document, project.Name));
+            }
+
+            return diagnostics;
+        }
     }
 }