Merge remote-tracking branch 'upstream/master' into IsCompilationEndAnalyzer

src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs

@@ -372,6 +372,7 @@ public async Task TestSynchronizeWithBuild()
                 ImmutableDictionary<ProjectId, ImmutableArray<DiagnosticData>>.Empty.Add(
                     document.Project.Id,
                     ImmutableArray.Create(DiagnosticData.Create(Diagnostic.Create(NoNameAnalyzer.s_syntaxRule, location), document.Project))),
+                new TaskQueue(service.Listener, TaskScheduler.Default),
                 onBuildCompleted: true,
                 CancellationToken.None);
 

src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService_BuildSynchronization.cs

@@ -7,36 +7,26 @@
 using System.Collections.Immutable;
 using System.Threading;
 using System.Threading.Tasks;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.Diagnostics
 {
     internal partial class DiagnosticAnalyzerService
     {
-        /// <summary>
-        /// Initialize solution state for synchronizing build errors with live errors.
-        /// 
-        /// no cancellationToken since this can't be cancelled
-        /// </summary>
-        public Task InitializeSynchronizationStateWithBuildAsync(Solution solution, CancellationToken cancellationToken)
-        {
-            if (_map.TryGetValue(solution.Workspace, out var analyzer))
-            {
-                return analyzer.InitializeSynchronizationStateWithBuildAsync(solution, cancellationToken);
-            }
-
-            return Task.CompletedTask;
-        }
-
         /// <summary>
         /// Synchronize build errors with live error.
-        /// 
-        /// no cancellationToken since this can't be cancelled
         /// </summary>
-        public Task SynchronizeWithBuildAsync(Workspace workspace, ImmutableDictionary<ProjectId, ImmutableArray<DiagnosticData>> diagnostics, bool onBuildCompleted, CancellationToken cancellationToken)
+        public Task SynchronizeWithBuildAsync(
+            Workspace workspace,
+            ImmutableDictionary<ProjectId,
+            ImmutableArray<DiagnosticData>> diagnostics,
+            TaskQueue postBuildAndErrorListRefreshTaskQueue,
+            bool onBuildCompleted,
+            CancellationToken cancellationToken)
         {
             if (_map.TryGetValue(workspace, out var analyzer))
             {
-                return analyzer.SynchronizeWithBuildAsync(diagnostics, onBuildCompleted, cancellationToken);
+                return analyzer.SynchronizeWithBuildAsync(diagnostics, postBuildAndErrorListRefreshTaskQueue, onBuildCompleted, cancellationToken);
             }
 
             return Task.CompletedTask;

src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.ProjectState.cs

@@ -187,7 +187,13 @@ public async Task<DiagnosticAnalysisResult> GetProjectAnalysisDataAsync(IPersist
                 return builder.ToResult();
             }
 
-            public async Task SaveAsync(IPersistentStorageService persistentService, Project project, DiagnosticAnalysisResult result)
+            public Task SaveAsync(IPersistentStorageService persistentService, Project project, DiagnosticAnalysisResult result)
+                => SaveCoreAsync(project, result, persistentService);
+
+            public Task SaveInMemoryCacheAsync(Project project, DiagnosticAnalysisResult result)
+                => SaveCoreAsync(project, result, persistentService: null);
+
+            private async Task SaveCoreAsync(Project project, DiagnosticAnalysisResult result, IPersistentStorageService? persistentService)
             {
                 Contract.ThrowIfTrue(result.IsAggregatedForm);
                 Contract.ThrowIfNull(result.DocumentIds);
@@ -343,12 +349,13 @@ private async Task<DiagnosticAnalysisResult> LoadInitialProjectAnalysisDataAsync
                 return builder.ToResult();
             }
 
-            private async Task SerializeAsync(IPersistentStorageService persistentService, DiagnosticDataSerializer serializer, Project project, TextDocument? document, object key, string stateKey, ImmutableArray<DiagnosticData> diagnostics)
+            private async Task SerializeAsync(IPersistentStorageService? persistentService, DiagnosticDataSerializer serializer, Project project, TextDocument? document, object key, string stateKey, ImmutableArray<DiagnosticData> diagnostics)
             {
                 Contract.ThrowIfFalse(document == null || document.Project == project);
 
                 // try to serialize it
-                if (await serializer.SerializeAsync(persistentService, project, document, stateKey, diagnostics, CancellationToken.None).ConfigureAwait(false))
+                if (persistentService != null &&
+                    await serializer.SerializeAsync(persistentService, project, document, stateKey, diagnostics, CancellationToken.None).ConfigureAwait(false))
                 {
                     // we succeeded saving it to persistent storage. remove it from in memory cache if it exists
                     RemoveInMemoryCacheEntry(key, stateKey);

src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_BuildSynchronization.cs

@@ -12,28 +12,21 @@
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Internal.Log;
 using Microsoft.CodeAnalysis.Options;
+using Microsoft.CodeAnalysis.PooledObjects;
+using Microsoft.CodeAnalysis.SolutionCrawler;
 using Microsoft.CodeAnalysis.Workspaces.Diagnostics;
-
-#if NETSTANDARD2_0
 using Roslyn.Utilities;
-#endif
 
 namespace Microsoft.CodeAnalysis.Diagnostics.EngineV2
 {
     internal partial class DiagnosticIncrementalAnalyzer
     {
-        public async Task InitializeSynchronizationStateWithBuildAsync(Solution solution, CancellationToken cancellationToken)
-        {
-            foreach (var project in solution.Projects)
-            {
-                cancellationToken.ThrowIfCancellationRequested();
-
-                var stateSets = _stateManager.CreateBuildOnlyProjectStateSet(project);
-                _ = await ProjectAnalysisData.CreateAsync(PersistentStorageService, project, stateSets, avoidLoadingData: false, cancellationToken).ConfigureAwait(false);
-            }
-        }
-
-        public async Task SynchronizeWithBuildAsync(ImmutableDictionary<ProjectId, ImmutableArray<DiagnosticData>> buildDiagnostics, bool onBuildCompleted, CancellationToken cancellationToken)
+        public async Task SynchronizeWithBuildAsync(
+            ImmutableDictionary<ProjectId,
+            ImmutableArray<DiagnosticData>> buildDiagnostics,
+            TaskQueue postBuildAndErrorListRefreshTaskQueue,
+            bool onBuildCompleted,
+            CancellationToken cancellationToken)
         {
             var options = Workspace.Options;
 
@@ -59,37 +52,65 @@ public async Task SynchronizeWithBuildAsync(ImmutableDictionary<ProjectId, Immut
                         continue;
                     }
 
-                    // REVIEW: do build diagnostics include suppressed diagnostics?
                     var stateSets = _stateManager.CreateBuildOnlyProjectStateSet(project);
+                    var newResult = CreateAnalysisResults(project, stateSets, diagnostics);
 
-                    // We load data since we don't know right version.
-                    var oldAnalysisData = await ProjectAnalysisData.CreateAsync(PersistentStorageService, project, stateSets, avoidLoadingData: false, cancellationToken).ConfigureAwait(false);
-                    var newResult = CreateAnalysisResults(project, stateSets, oldAnalysisData, diagnostics);
-
+                    // PERF: Save the diagnostics into in-memory cache on the main thread.
+                    //       Saving them into persistent storage is expensive, so we invoke that operation on a separate task queue
+                    //       to ensure faster error list refresh.
                     foreach (var stateSet in stateSets)
                     {
                         cancellationToken.ThrowIfCancellationRequested();
 
                         var state = stateSet.GetOrCreateProjectState(project.Id);
                         var result = GetResultOrEmpty(newResult, stateSet.Analyzer, project.Id, VersionStamp.Default);
-                        await state.SaveAsync(PersistentStorageService, project, result).ConfigureAwait(false);
+
+                        // Save into in-memory cache.
+                        await state.SaveInMemoryCacheAsync(project, result).ConfigureAwait(false);
+
+                        // Save into persistent storage on a separate post-build and post error list refresh task queue.
+                        _ = postBuildAndErrorListRefreshTaskQueue.ScheduleTask(
+                                nameof(SynchronizeWithBuildAsync),
+                                () => state.SaveAsync(PersistentStorageService, project, result),
+                                cancellationToken);
                     }
 
-                    // REVIEW: this won't handle active files. might need to tweak it later.
-                    RaiseProjectDiagnosticsIfNeeded(project, stateSets, oldAnalysisData.Result, newResult);
+                    // Raise diagnostic updated events after the new diagnostics have been stored into the in-memory cache.
+                    if (diagnostics.IsEmpty)
+                    {
+                        ClearAllDiagnostics(stateSets, projectId);
+                    }
+                    else
+                    {
+                        RaiseProjectDiagnosticsIfNeeded(project, stateSets, newResult);
+                    }
                 }
 
-                // Refresh diagnostics for open files after solution build completes.
+                // Refresh live diagnostics after solution build completes.
                 if (onBuildCompleted && PreferLiveErrorsOnOpenedFiles(options))
                 {
-                    // Enqueue re-analysis of active document.
+                    // Enqueue re-analysis of active document with high-priority right away.
                     if (_documentTrackingService?.GetActiveDocument(solution) is { } activeDocument)
                     {
                         AnalyzerService.Reanalyze(Workspace, documentIds: ImmutableArray.Create(activeDocument.Id), highPriority: true);
                     }
 
-                    // Enqueue re-analysis of open documents.
-                    AnalyzerService.Reanalyze(Workspace, documentIds: Workspace.GetOpenDocumentIds(), highPriority: true);
+                    // Enqueue remaining re-analysis with normal priority on a separate task queue
+                    // that will execute at the end of all the post build and error list refresh tasks.
+                    _ = postBuildAndErrorListRefreshTaskQueue.ScheduleTask(nameof(SynchronizeWithBuildAsync), () =>
+                    {
+                        // Enqueue re-analysis of open documents.
+                        AnalyzerService.Reanalyze(Workspace, documentIds: Workspace.GetOpenDocumentIds());
+
+                        // Enqueue re-analysis of projects, if required.
+                        foreach (var projectsByLanguage in solution.Projects.GroupBy(p => p.Language))
+                        {
+                            if (SolutionCrawlerOptions.GetBackgroundAnalysisScope(Workspace.Options, projectsByLanguage.Key) == BackgroundAnalysisScope.FullSolution)
+                            {
+                                AnalyzerService.Reanalyze(Workspace, projectsByLanguage.Select(p => p.Id));
+                            }
+                        }
+                    }, cancellationToken);
                 }
             }
         }
@@ -107,22 +128,24 @@ private static void DebugVerifyDiagnosticLocations(ImmutableDictionary<ProjectId
         }
 
         private ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult> CreateAnalysisResults(
-            Project project, ImmutableArray<StateSet> stateSets, ProjectAnalysisData oldAnalysisData, ImmutableArray<DiagnosticData> diagnostics)
+            Project project, ImmutableArray<StateSet> stateSets, ImmutableArray<DiagnosticData> diagnostics)
         {
             using var poolObject = SharedPools.Default<HashSet<string>>().GetPooledObject();
 
             var lookup = diagnostics.ToLookup(d => d.Id);
 
             var builder = ImmutableDictionary.CreateBuilder<DiagnosticAnalyzer, DiagnosticAnalysisResult>();
+            using var _ = PooledHashSet<DocumentId>.GetInstance(out var existingDocumentsInStateSet);
             foreach (var stateSet in stateSets)
             {
                 var descriptors = DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(stateSet.Analyzer);
+                var liveDiagnostics = ConvertToLiveDiagnostics(lookup, descriptors, poolObject.Object);
 
-                var liveDiagnostics = MergeDiagnostics(
-                    ConvertToLiveDiagnostics(lookup, descriptors, poolObject.Object),
-                    oldAnalysisData.GetResult(stateSet.Analyzer).GetAllDiagnostics());
+                // Ensure that all documents with diagnostics in the previous state set are added to the result.
+                existingDocumentsInStateSet.Clear();
+                stateSet.CollectDocumentsWithDiagnostics(project.Id, existingDocumentsInStateSet);
 
-                builder.Add(stateSet.Analyzer, DiagnosticAnalysisResult.CreateFromBuild(project, liveDiagnostics));
+                builder.Add(stateSet.Analyzer, DiagnosticAnalysisResult.CreateFromBuild(project, liveDiagnostics, existingDocumentsInStateSet));
             }
 
             return builder.ToImmutable();
@@ -134,26 +157,6 @@ private static bool PreferBuildErrors(OptionSet options)
         private static bool PreferLiveErrorsOnOpenedFiles(OptionSet options)
             => options.GetOption(InternalDiagnosticsOptions.PreferLiveErrorsOnOpenedFiles);
 
-        private static ImmutableArray<DiagnosticData> MergeDiagnostics(ImmutableArray<DiagnosticData> newDiagnostics, ImmutableArray<DiagnosticData> existingDiagnostics)
-        {
-            ImmutableArray<DiagnosticData>.Builder? builder = null;
-
-            if (newDiagnostics.Length > 0)
-            {
-                builder = ImmutableArray.CreateBuilder<DiagnosticData>();
-                builder.AddRange(newDiagnostics);
-            }
-
-            if (existingDiagnostics.Length > 0)
-            {
-                // retain hidden live diagnostics since it won't be comes from build.
-                builder ??= ImmutableArray.CreateBuilder<DiagnosticData>();
-                builder.AddRange(existingDiagnostics.Where(d => d.Severity == DiagnosticSeverity.Hidden));
-            }
-
-            return builder == null ? ImmutableArray<DiagnosticData>.Empty : builder.ToImmutable();
-        }
-
         private static ImmutableArray<DiagnosticData> ConvertToLiveDiagnostics(
             ILookup<string, DiagnosticData> lookup, ImmutableArray<DiagnosticDescriptor> descriptors, HashSet<string> seen)
         {

src/VisualStudio/Core/Def/Implementation/TaskList/ExternalErrorDiagnosticUpdateSource.cs

@@ -50,6 +50,16 @@ internal sealed class ExternalErrorDiagnosticUpdateSource : IDiagnosticUpdateSou
         /// </summary>
         private readonly TaskQueue _taskQueue;
 
+        /// <summary>
+        /// Task queue to serialize all the post-build and post error list refresh tasks.
+        /// Error list refresh requires build/live diagnostics de-duping to complete, which happens during
+        /// <see cref="SyncBuildErrorsAndReportOnBuildCompletedAsync(DiagnosticAnalyzerService, InProgressState, CancellationToken)"/>.
+        /// Computationally expensive tasks such as writing build errors into persistent storage,
+        /// invoking background analysis on open files/solution after build completes, etc.
+        /// are added to this task queue to help ensure faster error list refresh.
+        /// </summary>
+        private readonly TaskQueue _postBuildAndErrorListRefreshTaskQueue;
+
         // Gate for concurrent access and fields guarded with this gate.
         private readonly object _gate = new();
         private InProgressState? _stateDoNotAccessDirectly;
@@ -84,6 +94,7 @@ internal sealed class ExternalErrorDiagnosticUpdateSource : IDiagnosticUpdateSou
         {
             // use queue to serialize work. no lock needed
             _taskQueue = new TaskQueue(listener, TaskScheduler.Default);
+            _postBuildAndErrorListRefreshTaskQueue = new TaskQueue(listener, TaskScheduler.Default);
             _disposalToken = disposalToken;
 
             _workspace = workspace;
@@ -212,17 +223,12 @@ async Task ClearErrorsCoreAsync(ProjectId projectId, Solution solution, InProgre
 
         private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e)
         {
+            // Clear relevant build-only errors on workspace events such as solution added/removed/reloaded,
+            // project added/removed/reloaded, etc.
             switch (e.Kind)
             {
                 case WorkspaceChangeKind.SolutionAdded:
-                    _taskQueue.ScheduleTask("OnSolutionAdded", async () =>
-                    {
-                        e.OldSolution.ProjectIds.Do(p => ClearBuildOnlyProjectErrors(e.OldSolution, p));
-                        if (_diagnosticService is DiagnosticAnalyzerService diagnosticAnalyzerService)
-                        {
-                            await diagnosticAnalyzerService.InitializeSynchronizationStateWithBuildAsync(e.NewSolution, _disposalToken).ConfigureAwait(false);
-                        }
-                    }, _disposalToken);
+                    _taskQueue.ScheduleTask("OnSolutionAdded", () => e.OldSolution.ProjectIds.Do(p => ClearBuildOnlyProjectErrors(e.OldSolution, p)), _disposalToken);
                     break;
 
                 case WorkspaceChangeKind.SolutionRemoved:
@@ -276,32 +282,39 @@ internal void OnSolutionBuildCompleted()
             // Enqueue build/live sync in the queue.
             _taskQueue.ScheduleTask("OnSolutionBuild", async () =>
             {
-                // nothing to do
-                if (inProgressState == null)
+                try
                 {
-                    return;
-                }
+                    // nothing to do
+                    if (inProgressState == null)
+                    {
+                        return;
+                    }
 
-                // Explicitly start solution crawler if it didn't start yet. since solution crawler is lazy, 
-                // user might have built solution before workspace fires its first event yet (which is when solution crawler is initialized)
-                // here we give initializeLazily: false so that solution crawler is fully initialized when we do de-dup live and build errors,
-                // otherwise, we will think none of error we have here belong to live errors since diagnostic service is not initialized yet.
-                var registrationService = (SolutionCrawlerRegistrationService)_workspace.Services.GetRequiredService<ISolutionCrawlerRegistrationService>();
-                registrationService.EnsureRegistration(_workspace, initializeLazily: false);
+                    // Explicitly start solution crawler if it didn't start yet. since solution crawler is lazy, 
+                    // user might have built solution before workspace fires its first event yet (which is when solution crawler is initialized)
+                    // here we give initializeLazily: false so that solution crawler is fully initialized when we do de-dup live and build errors,
+                    // otherwise, we will think none of error we have here belong to live errors since diagnostic service is not initialized yet.
+                    var registrationService = (SolutionCrawlerRegistrationService)_workspace.Services.GetRequiredService<ISolutionCrawlerRegistrationService>();
+                    registrationService.EnsureRegistration(_workspace, initializeLazily: false);
 
-                // Mark the status as updated to refresh error list before we invoke 'SyncBuildErrorsAndReportAsync', which can take some time to complete.
-                OnBuildProgressChanged(inProgressState, BuildProgress.Updated);
+                    // Mark the status as updated to refresh error list before we invoke 'SyncBuildErrorsAndReportAsync', which can take some time to complete.
+                    OnBuildProgressChanged(inProgressState, BuildProgress.Updated);
 
-                // We are about to update live analyzer data using one from build.
-                // pause live analyzer
-                using var operation = _notificationService.Start("BuildDone");
-                if (_diagnosticService is DiagnosticAnalyzerService diagnosticService)
+                    // We are about to update live analyzer data using one from build.
+                    // pause live analyzer
+                    using var operation = _notificationService.Start("BuildDone");
+                    if (_diagnosticService is DiagnosticAnalyzerService diagnosticService)
+                    {
+                        await SyncBuildErrorsAndReportOnBuildCompletedAsync(diagnosticService, inProgressState, cancellationToken).ConfigureAwait(false);
+                    }
+
+                    // Mark build as complete.
+                    OnBuildProgressChanged(inProgressState, BuildProgress.Done);
+                }
+                finally
                 {
-                    await SyncBuildErrorsAndReportOnBuildCompletedAsync(diagnosticService, inProgressState, cancellationToken).ConfigureAwait(false);
+                    await _postBuildAndErrorListRefreshTaskQueue.LastScheduledTask.ConfigureAwait(false);
                 }
-
-                // Mark build as complete.
-                OnBuildProgressChanged(inProgressState, BuildProgress.Done);
             }, cancellationToken);
         }
 
@@ -336,7 +349,7 @@ private async Task SyncBuildErrorsAndReportOnBuildCompletedAsync(DiagnosticAnaly
             }
 
             // Report pending live errors
-            await diagnosticService.SynchronizeWithBuildAsync(_workspace, pendingLiveErrorsToSync, onBuildCompleted: true, cancellationToken).ConfigureAwait(false);
+            await diagnosticService.SynchronizeWithBuildAsync(_workspace, pendingLiveErrorsToSync, _postBuildAndErrorListRefreshTaskQueue, onBuildCompleted: true, cancellationToken).ConfigureAwait(false);
         }
 
         private void ReportBuildErrors<T>(T item, Solution solution, ImmutableArray<DiagnosticData> buildErrors)
@@ -446,7 +459,7 @@ private async Task SetLiveErrorsForProjectAsync(ProjectId projectId, ImmutableAr
             {
                 // make those errors live errors
                 var map = ProjectErrorMap.Empty.Add(projectId, diagnostics);
-                await diagnosticAnalyzerService.SynchronizeWithBuildAsync(_workspace, map, onBuildCompleted: false, cancellationToken).ConfigureAwait(false);
+                await diagnosticAnalyzerService.SynchronizeWithBuildAsync(_workspace, map, _postBuildAndErrorListRefreshTaskQueue, onBuildCompleted: false, cancellationToken).ConfigureAwait(false);
             }
         }
 

src/Workspaces/Core/Portable/Diagnostics/DiagnosticAnalysisResult.cs

@@ -2,6 +2,7 @@
 // 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.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
@@ -101,13 +102,15 @@ public static DiagnosticAnalysisResult CreateInitialResult(ProjectId projectId)
                 fromBuild: false);
         }
 
-        public static DiagnosticAnalysisResult CreateFromBuild(Project project, ImmutableArray<DiagnosticData> diagnostics)
+        public static DiagnosticAnalysisResult CreateFromBuild(Project project, ImmutableArray<DiagnosticData> diagnostics, IEnumerable<DocumentId> initialDocuments)
         {
             // we can't distinguish locals and non locals from build diagnostics nor determine right snapshot version for the build.
             // so we put everything in as semantic local with default version. this lets us to replace those to live diagnostics when needed easily.
             var version = VersionStamp.Default;
 
             var documentIds = ImmutableHashSet.CreateBuilder<DocumentId>();
+            documentIds.AddRange(initialDocuments);
+
             var diagnosticsWithDocumentId = PooledDictionary<DocumentId, ArrayBuilder<DiagnosticData>>.GetInstance();
             var diagnosticsWithoutDocumentId = ArrayBuilder<DiagnosticData>.GetInstance();