Merge pull request #30593 from mavasani/PerfIssues

Fix for perf issues in CompilationWithAnalyzers found from perf traces

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisState.cs

@@ -20,7 +20,7 @@ namespace Microsoft.CodeAnalysis.Diagnostics
     /// </summary>
     internal partial class AnalysisState
     {
-        private readonly object _gate;
+        private readonly SemaphoreSlim _gate;
 
         /// <summary>
         /// Per-analyzer analysis state map.
@@ -61,7 +61,7 @@ internal partial class AnalysisState
 
         public AnalysisState(ImmutableArray<DiagnosticAnalyzer> analyzers, CompilationData compilationData, CompilationOptions compilationOptions)
         {
-            _gate = new object();
+            _gate = new SemaphoreSlim(initialCount: 1);
             _analyzerStateMap = CreateAnalyzerStateMap(analyzers, out _analyzerStates);
             _compilationData = compilationData;
             _compilationOptions = compilationOptions;
@@ -135,7 +135,7 @@ private PerAnalyzerState GetAnalyzerState(DiagnosticAnalyzer analyzer)
             AnalyzerDriver driver,
             CancellationToken cancellationToken)
         {
-            lock (_gate)
+            using (_gate.DisposableWait(cancellationToken))
             {
                 if (_treesWithGeneratedSourceEvents.Contains(tree))
                 {
@@ -147,7 +147,7 @@ private PerAnalyzerState GetAnalyzerState(DiagnosticAnalyzer analyzer)
             var symbols = GetDeclaredSymbolsInTree(tree, compilation, getCachedSemanticModel, cancellationToken);
             var compilationEvents = CreateCompilationEventsForTree(symbols.Concat(globalNs), tree, compilation);
 
-            lock (_gate)
+            using (_gate.DisposableWait(cancellationToken))
             {
                 if (_treesWithGeneratedSourceEvents.Contains(tree))
                 {
@@ -191,7 +191,7 @@ private static ImmutableArray<CompilationEvent> CreateCompilationEventsForTree(I
 
         private void GenerateSimulatedCompilationNonSourceEvent(Compilation compilation, AnalyzerDriver driver, bool started, CancellationToken cancellationToken)
         {
-            lock (_gate)
+            using (_gate.DisposableWait(cancellationToken))
             {
                 var eventAlreadyGenerated = started ? _compilationStartGenerated : _compilationEndGenerated;
                 if (eventAlreadyGenerated)
@@ -218,7 +218,7 @@ public async Task OnCompilationEventsGeneratedAsync(ImmutableArray<CompilationEv
         {
             await EnsureAnalyzerActionCountsInitializedAsync(driver, cancellationToken).ConfigureAwait(false);
 
-            lock (_gate)
+            using (_gate.DisposableWait(cancellationToken))
             {
                 OnCompilationEventsGenerated_NoLock(compilationEvents, filterTreeOpt: null, driver: driver, cancellationToken: cancellationToken);
             }
@@ -479,7 +479,8 @@ public async Task OnCompilationEventProcessedAsync(CompilationEvent compilationE
             }
 
             // Remove the event from event map.
-            lock (_gate)
+            // Note: We do not pass in the cancellationToken to DisposableWait to ensure the state is updated.
+            using (_gate.DisposableWait())
             {
                 UpdateEventsMap_NoLock(compilationEvent, add: false);
             }
@@ -488,9 +489,9 @@ public async Task OnCompilationEventProcessedAsync(CompilationEvent compilationE
         /// <summary>
         /// Gets pending events for given set of analyzers for the given syntax tree.
         /// </summary>
-        public ImmutableArray<CompilationEvent> GetPendingEvents(ImmutableArray<DiagnosticAnalyzer> analyzers, SyntaxTree tree)
+        public ImmutableArray<CompilationEvent> GetPendingEvents(ImmutableArray<DiagnosticAnalyzer> analyzers, SyntaxTree tree, CancellationToken cancellationToken)
         {
-            lock (_gate)
+            using (_gate.DisposableWait(cancellationToken))
             {
                 return GetPendingEvents_NoLock(analyzers, tree);
             }
@@ -544,9 +545,14 @@ private ImmutableArray<CompilationEvent> GetPendingEvents_NoLock(ImmutableArray<
         /// <param name="analyzers"></param>
         /// <param name="includeSourceEvents">Indicates if source events (symbol declared, compilation unit completed event) should be included.</param>
         /// <param name="includeNonSourceEvents">Indicates if compilation wide events (compilation started and completed event) should be included.</param>
-        public ImmutableArray<CompilationEvent> GetPendingEvents(ImmutableArray<DiagnosticAnalyzer> analyzers, bool includeSourceEvents, bool includeNonSourceEvents)
+        /// <param name="cancellationToken">Cancellation token.</param>
+        public ImmutableArray<CompilationEvent> GetPendingEvents(
+            ImmutableArray<DiagnosticAnalyzer> analyzers,
+            bool includeSourceEvents,
+            bool includeNonSourceEvents,
+            CancellationToken cancellationToken)
         {
-            lock (_gate)
+            using (_gate.DisposableWait(cancellationToken))
             {
                 return GetPendingEvents_NoLock(analyzers, includeSourceEvents, includeNonSourceEvents);
             }
@@ -628,11 +634,11 @@ public bool HasPendingSyntaxAnalysis(AnalysisScope analysisScope)
         /// <summary>
         /// Returns true if we have any pending symbol analysis for given analysis scope.
         /// </summary>
-        public bool HasPendingSymbolAnalysis(AnalysisScope analysisScope)
+        public bool HasPendingSymbolAnalysis(AnalysisScope analysisScope, CancellationToken cancellationToken)
         {
             Debug.Assert(analysisScope.FilterTreeOpt != null);
 
-            var symbolDeclaredEvents = GetPendingSymbolDeclaredEvents(analysisScope.FilterTreeOpt);
+            var symbolDeclaredEvents = GetPendingSymbolDeclaredEvents(analysisScope.FilterTreeOpt, cancellationToken);
             foreach (var symbolDeclaredEvent in symbolDeclaredEvents)
             {
                 if (analysisScope.ShouldAnalyze(symbolDeclaredEvent.Symbol))
@@ -651,11 +657,11 @@ public bool HasPendingSymbolAnalysis(AnalysisScope analysisScope)
             return false;
         }
 
-        private ImmutableArray<SymbolDeclaredCompilationEvent> GetPendingSymbolDeclaredEvents(SyntaxTree tree)
+        private ImmutableArray<SymbolDeclaredCompilationEvent> GetPendingSymbolDeclaredEvents(SyntaxTree tree, CancellationToken cancellationToken)
         {
             Debug.Assert(tree != null);
 
-            lock (_gate)
+            using (_gate.DisposableWait(cancellationToken))
             {
                 HashSet<CompilationEvent> compilationEvents;
                 if (!_pendingSourceEvents.TryGetValue(tree, out compilationEvents))

src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilationWithAnalyzers.cs

@@ -355,7 +355,7 @@ private async Task<ImmutableArray<Diagnostic>> GetAnalyzerCompilationDiagnostics
             var diagnostics = ImmutableArray<Diagnostic>.Empty;
             var analysisScope = new AnalysisScope(_compilation, analyzers, _analysisOptions.ConcurrentAnalysis, categorizeDiagnostics: true);
             Func<AsyncQueue<CompilationEvent>> getEventQueue = () =>
-               GetPendingEvents(analyzers, includeSourceEvents: true, includeNonSourceEvents: true);
+               GetPendingEvents(analyzers, includeSourceEvents: true, includeNonSourceEvents: true, cancellationToken);
 
             // Compute the analyzer diagnostics for the given analysis scope.
             await ComputeAnalyzerDiagnosticsAsync(analysisScope, getEventQueue, newTaskToken: 0, cancellationToken: cancellationToken).ConfigureAwait(false);
@@ -580,18 +580,20 @@ private async Task<ImmutableArray<Diagnostic>> GetAnalyzerSemanticDiagnosticsCor
                 {
                     var pendingAnalysisScope = pendingAnalyzers.Length < analyzers.Length ? analysisScope.WithAnalyzers(pendingAnalyzers) : analysisScope;
 
-                    Func<AsyncQueue<CompilationEvent>> getEventQueue = () => GetPendingEvents(analyzers, model.SyntaxTree);
+                    Func<AsyncQueue<CompilationEvent>> getEventQueue = () => GetPendingEvents(analyzers, model.SyntaxTree, cancellationToken);
 
                     // Compute the analyzer diagnostics for the given analysis scope.
                     // We need to loop till symbol analysis is complete for any partial symbols being processed for other tree diagnostic requests.
                     do
                     {
+                        cancellationToken.ThrowIfCancellationRequested();
+
                         (ImmutableArray<CompilationEvent> compilationEvents, bool hasSymbolStartActions) = await ComputeAnalyzerDiagnosticsAsync(pendingAnalysisScope, getEventQueue, taskToken, cancellationToken).ConfigureAwait(false);
                         if (hasSymbolStartActions)
                         {
                             await processPartialSymbolLocationsAsync(compilationEvents, analysisScope).ConfigureAwait(false);
                         }
-                    } while (_analysisOptions.ConcurrentAnalysis && _analysisState.HasPendingSymbolAnalysis(pendingAnalysisScope));
+                    } while (_analysisOptions.ConcurrentAnalysis && _analysisState.HasPendingSymbolAnalysis(pendingAnalysisScope, cancellationToken));
 
                     if (_analysisOptions.ConcurrentAnalysis)
                     {
@@ -642,12 +644,24 @@ async Task processPartialSymbolLocationsAsync(ImmutableArray<CompilationEvent> c
 
                 if (partialTrees != null)
                 {
-                    await Task.WhenAll(partialTrees.Select(t =>
-                        Task.Run(() =>
+                    if (AnalysisOptions.ConcurrentAnalysis)
+                    {
+                        await Task.WhenAll(partialTrees.Select(tree =>
+                            Task.Run(() =>
+                            {
+                                var treeModel = _compilationData.GetOrCreateCachedSemanticModel(tree, _compilation, cancellationToken);
+                                return GetAnalyzerSemanticDiagnosticsAsync(treeModel, filterSpan: null, cancellationToken);
+                            }, cancellationToken))).ConfigureAwait(false);
+                    }
+                    else
+                    {
+                        foreach (var tree in partialTrees)
                         {
-                            var treeModel = _compilationData.GetOrCreateCachedSemanticModel(t, _compilation, cancellationToken);
-                            return GetAnalyzerSemanticDiagnosticsAsync(treeModel, filterSpan: null, cancellationToken);
-                        }))).ConfigureAwait(false);
+                            cancellationToken.ThrowIfCancellationRequested();
+                            var treeModel = _compilationData.GetOrCreateCachedSemanticModel(tree, _compilation, cancellationToken);
+                            await GetAnalyzerSemanticDiagnosticsAsync(treeModel, filterSpan: null, cancellationToken).ConfigureAwait(false);
+                        }
+                    }
                 }
             }
         }
@@ -1062,13 +1076,13 @@ private void ClearExecutingTask(Task computeTask, SyntaxTree treeOpt)
             }
         }
 
-        private AsyncQueue<CompilationEvent> GetPendingEvents(ImmutableArray<DiagnosticAnalyzer> analyzers, SyntaxTree tree)
+        private AsyncQueue<CompilationEvent> GetPendingEvents(ImmutableArray<DiagnosticAnalyzer> analyzers, SyntaxTree tree, CancellationToken cancellationToken)
         {
             var eventQueue = _eventQueuePool.Allocate();
             Debug.Assert(!eventQueue.IsCompleted);
             Debug.Assert(eventQueue.Count == 0);
 
-            foreach (var compilationEvent in _analysisState.GetPendingEvents(analyzers, tree))
+            foreach (var compilationEvent in _analysisState.GetPendingEvents(analyzers, tree, cancellationToken))
             {
                 eventQueue.TryEnqueue(compilationEvent);
             }
@@ -1076,7 +1090,11 @@ private AsyncQueue<CompilationEvent> GetPendingEvents(ImmutableArray<DiagnosticA
             return eventQueue;
         }
 
-        private AsyncQueue<CompilationEvent> GetPendingEvents(ImmutableArray<DiagnosticAnalyzer> analyzers, bool includeSourceEvents, bool includeNonSourceEvents)
+        private AsyncQueue<CompilationEvent> GetPendingEvents(
+            ImmutableArray<DiagnosticAnalyzer> analyzers,
+            bool includeSourceEvents,
+            bool includeNonSourceEvents,
+            CancellationToken cancellationToken)
         {
             Debug.Assert(includeSourceEvents || includeNonSourceEvents);
 
@@ -1084,7 +1102,7 @@ private AsyncQueue<CompilationEvent> GetPendingEvents(ImmutableArray<DiagnosticA
             Debug.Assert(!eventQueue.IsCompleted);
             Debug.Assert(eventQueue.Count == 0);
 
-            foreach (var compilationEvent in _analysisState.GetPendingEvents(analyzers, includeSourceEvents, includeNonSourceEvents))
+            foreach (var compilationEvent in _analysisState.GetPendingEvents(analyzers, includeSourceEvents, includeNonSourceEvents, cancellationToken))
             {
                 eventQueue.TryEnqueue(compilationEvent);
             }