Address review feedback

src/EditorFeatures/Core/Implementation/CodeFixes/CodeFixService.cs

@@ -294,30 +294,13 @@ public async Task<Document> ApplyCodeFixesForSpecificDiagnosticIdAsync(Document
             }
 
             var extensionManager = document.Project.Solution.Workspace.Services.GetService<IExtensionManager>();
-            var isPerProviderLoggingEnabled = RoslynEventSource.Instance.IsEnabled(EventLevel.Informational, EventKeywords.None);
 
             // run each CodeFixProvider to gather individual CodeFixes for reported diagnostics
             foreach (var fixer in allFixers.Distinct())
             {
                 cancellationToken.ThrowIfCancellationRequested();
 
-                if (isPerProviderLoggingEnabled)
-                {
-                    using (RoslynEventSource.LogInformationalBlock(FunctionId.CodeFixes_GetCodeFixesAsync, fixer.ToString(), cancellationToken))
-                    {
-                        await ProcessFixerAsync(fixer).ConfigureAwait(false);
-                    }
-                }
-                else
-                {
-                    await ProcessFixerAsync(fixer).ConfigureAwait(false);
-                }
-
-                // Just need the first result if we are doing fix all in span
-                if (fixAllForInSpan && result.Any()) return;
-            }
-
-            async Task ProcessFixerAsync(CodeFixProvider fixer)
+                using (RoslynEventSource.LogInformationalBlock(FunctionId.CodeFixes_GetCodeFixesAsync, fixer, cancellationToken))
                 {
                     await AppendFixesOrConfigurationsAsync(
                         document, span, diagnostics, fixAllForInSpan, result, fixer,
@@ -336,6 +319,10 @@ async Task ProcessFixerAsync(CodeFixProvider fixer)
                         },
                         cancellationToken: cancellationToken).ConfigureAwait(false);
                 }
+
+                // Just need the first result if we are doing fix all in span
+                if (fixAllForInSpan && result.Any()) return;
+            }
         }
 
         private async Task<ImmutableArray<CodeFix>> GetCodeFixesAsync(
@@ -371,25 +358,10 @@ async Task ProcessFixerAsync(CodeFixProvider fixer)
                 return;
             }
 
-            var isPerProviderLoggingEnabled = RoslynEventSource.Instance.IsEnabled(EventLevel.Informational, EventKeywords.None);
-
             // append CodeFixCollection for each CodeFixProvider
             foreach (var provider in lazyConfigurationProviders.Value)
             {
-                if (isPerProviderLoggingEnabled)
-                {
-                    using (RoslynEventSource.LogInformationalBlock(FunctionId.CodeFixes_GetCodeFixesAsync, provider.ToString(), cancellationToken))
-                    {
-                        await AppendConfigurationsAsync(provider).ConfigureAwait(false);
-                    }
-                }
-                else
-                {
-                    await AppendConfigurationsAsync(provider).ConfigureAwait(false);
-                }
-            }
-
-            async Task AppendConfigurationsAsync(IConfigurationFixProvider provider)
+                using (RoslynEventSource.LogInformationalBlock(FunctionId.CodeFixes_GetCodeFixesAsync, provider, cancellationToken))
                 {
                     await AppendFixesOrConfigurationsAsync(
                         document, diagnosticsSpan, diagnostics, fixAllForInSpan: false, result, provider,
@@ -399,6 +371,7 @@ async Task AppendConfigurationsAsync(IConfigurationFixProvider provider)
                         cancellationToken: cancellationToken).ConfigureAwait(false);
                 }
             }
+        }
 
         private async Task AppendFixesOrConfigurationsAsync<TCodeFixProvider>(
             Document document,

src/Features/Core/Portable/CodeRefactorings/CodeRefactoringService.cs

@@ -114,26 +114,18 @@ private IEnumerable<CodeRefactoringProvider> GetProviders(Document document)
                 foreach (var provider in GetProviders(document))
                 {
                     tasks.Add(Task.Run(
-                        () => GetRefactoringsAsync(provider), cancellationToken));
-                }
-
-                var results = await Task.WhenAll(tasks).ConfigureAwait(false);
-                return results.WhereNotNull().ToImmutableArray();
-            }
-
-            Task<CodeRefactoring> GetRefactoringsAsync(CodeRefactoringProvider provider)
-            {
-                if (isPerProviderLoggingEnabled)
+                        () =>
                         {
-                    using (RoslynEventSource.LogInformationalBlock(FunctionId.Refactoring_CodeRefactoringService_GetRefactoringsAsync, provider.ToString(), cancellationToken))
+                            using (RoslynEventSource.LogInformationalBlock(FunctionId.Refactoring_CodeRefactoringService_GetRefactoringsAsync, provider, cancellationToken))
                             {
                                 return GetRefactoringFromProviderAsync(document, state, provider, extensionManager, isBlocking, cancellationToken);
                             }
+                        },
+                        cancellationToken));
                 }
-                else
-                {
-                    return GetRefactoringFromProviderAsync(document, state, provider, extensionManager, isBlocking, cancellationToken);
-                }
+
+                var results = await Task.WhenAll(tasks).ConfigureAwait(false);
+                return results.WhereNotNull().ToImmutableArray();
             }
         }
 

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

@@ -108,36 +108,12 @@ private class LatestDiagnosticsForSpanGetter
 
             public async Task<bool> TryGetAsync(List<DiagnosticData> list, CancellationToken cancellationToken)
             {
-                var containsFullResult = true;
-                var loggingEnabled = RoslynEventSource.Instance.IsEnabled(EventLevel.Informational, EventKeywords.None);
-
                 try
                 {
+                    var containsFullResult = true;
                     foreach (var stateSet in _stateSets)
                     {
-                        if (loggingEnabled)
-                        {
-                            using (RoslynEventSource.LogInformationalBlock(FunctionId.DiagnosticAnalyzerService_GetDiagnosticsForSpanAsync, stateSet.Analyzer.GetAnalyzerId(), cancellationToken))
-                            {
-                                await ProcessStateSetAsync(stateSet).ConfigureAwait(false);
-                            }
-                        }
-                        else
-                        {
-                            await ProcessStateSetAsync(stateSet).ConfigureAwait(false);
-                        }
-                    }
-
-                    // if we are blocked for data, then we should always have full result.
-                    Debug.Assert(!_blockForData || containsFullResult);
-                    return containsFullResult;
-                }
-                catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
-                {
-                    throw ExceptionUtilities.Unreachable;
-                }
-
-                async Task ProcessStateSetAsync(StateSet stateSet)
+                        using (RoslynEventSource.LogInformationalBlock(FunctionId.DiagnosticAnalyzerService_GetDiagnosticsForSpanAsync, stateSet.Analyzer, cancellationToken))
                         {
                             cancellationToken.ThrowIfCancellationRequested();
 
@@ -146,7 +122,7 @@ async Task ProcessStateSetAsync(StateSet stateSet)
                             // check whether compilation end code fix is enabled
                             if (!_document.Project.Solution.Workspace.Options.GetOption(InternalDiagnosticsOptions.CompilationEndCodeFix))
                             {
-                        return;
+                                continue;
                             }
 
                             // check whether heuristic is enabled
@@ -160,7 +136,7 @@ async Task ProcessStateSetAsync(StateSet stateSet)
                                 var version = await GetDiagnosticVersionAsync(_project, cancellationToken).ConfigureAwait(false);
                                 if (state.IsEmpty(_document.Id) || result.Version != version)
                                 {
-                            return;
+                                    continue;
                                 }
                             }
 
@@ -168,6 +144,16 @@ async Task ProcessStateSetAsync(StateSet stateSet)
                         }
                     }
 
+                    // if we are blocked for data, then we should always have full result.
+                    Debug.Assert(!_blockForData || containsFullResult);
+                    return containsFullResult;
+                }
+                catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
+                {
+                    throw ExceptionUtilities.Unreachable;
+                }
+            }
+
             private async Task<bool> TryGetSyntaxAndSemanticDiagnosticsAsync(StateSet stateSet, List<DiagnosticData> list, CancellationToken cancellationToken)
             {
                 // unfortunately, we need to special case compiler diagnostic analyzer so that

src/Workspaces/Core/Portable/Log/RoslynEventSource.LogBlock.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+#nullable enable
+
 using System;
+using System.Diagnostics;
 using System.Diagnostics.Tracing;
 using System.Threading;
-using Microsoft.CodeAnalysis.PooledObjects;
 
 namespace Microsoft.CodeAnalysis.Internal.Log
 {
     internal partial class RoslynEventSource
     {
-        // Regardless of how many tasks we can run in parallel on the machine, we likely won't need more than 256
-        // instrumentation points in flight at a given time.
-        // Use an object pool since we may be logging up to 1-10k events/second
-        private static readonly ObjectPool<RoslynLogBlock> s_pool = new ObjectPool<RoslynLogBlock>(() => new RoslynLogBlock(s_pool), Math.Min(Environment.ProcessorCount * 8, 256));
-
         /// <summary>
-        /// next unique block id that will be given to each LogBlock
+        /// Logs an informational block with given <paramref name="entity"/>'s <see cref="object.ToString"/> representation as the message
+        /// and specified <paramref name="functionId"/>.
+        /// On dispose of the returned disposable object, it logs the 'tick' count between the start and end of the block.
+        /// Unlike other logging methods on <see cref="RoslynEventSource"/>, this method does not check
+        /// if the specified <paramref name="functionId"/> was explicitly enabled.
+        /// Instead it checks if the <see cref="RoslynEventSource"/> was enabled at <see cref="EventLevel.Informational"/> level.
         /// </summary>
-        private static int s_lastUniqueBlockId;
+        public static IDisposable LogInformationalBlock(FunctionId functionId, object entity, CancellationToken cancellationToken)
+            => LogBlock.Create(functionId, entity, EventLevel.Informational, cancellationToken);
 
         /// <summary>
-        /// Logs a block with the given <paramref name="message"/> and specified <paramref name="functionId"/>.
+        /// Logs an informational message block with the given <paramref name="message"/>> and specified <paramref name="functionId"/>.
         /// On dispose of the returned disposable object, it logs the 'tick' count between the start and end of the block.
         /// Unlike other logging methods on <see cref="RoslynEventSource"/>, this method does not check
         /// if the specified <paramref name="functionId"/> was explicitly enabled.
         /// Instead it checks if the <see cref="RoslynEventSource"/> was enabled at <see cref="EventLevel.Informational"/> level.
         /// </summary>
         public static IDisposable LogInformationalBlock(FunctionId functionId, string message, CancellationToken cancellationToken)
-            => LogBlock(functionId, message, EventLevel.Informational, cancellationToken);
+            => LogBlock.Create(functionId, message, EventLevel.Informational, cancellationToken);
 
-        private static IDisposable LogBlock(FunctionId functionId, string message, EventLevel requiredEventLevel, CancellationToken cancellationToken)
+        /// <summary>
+        /// This tracks the logged message. On instantiation, it logs 'Started block' with other event data.
+        /// On dispose, it logs 'Ended block' with the same event data so we can track which block started and ended when looking at logs.
+        /// </summary>
+        private struct LogBlock : IDisposable
         {
-            if (!Instance.IsEnabled(requiredEventLevel, EventKeywords.None))
+            private readonly FunctionId _functionId;
+            private readonly object? _entityForMessage;
+            private readonly EventLevel _eventLevel;
+            private readonly int _blockId;
+            private readonly CancellationToken _cancellationToken;
+
+            private int _tick;
+            private bool _startLogged;
+            private string? _message;
+
+            /// <summary>
+            /// next unique block id that will be given to each LogBlock
+            /// </summary>
+            private static int s_lastUniqueBlockId;
+
+            private LogBlock(
+                FunctionId functionId,
+                string? message,
+                object? entityForMessage,
+                EventLevel eventLevel,
+                int blockId,
+                CancellationToken cancellationToken)
+            {
+                Debug.Assert(message != null || entityForMessage != null);
+
+                _functionId = functionId;
+                _message = message;
+                _entityForMessage = entityForMessage;
+                _eventLevel = eventLevel;
+                _blockId = blockId;
+                _cancellationToken = cancellationToken;
+                _tick = Environment.TickCount;
+                _startLogged = false;
+            }
+
+            public static LogBlock Create(
+                FunctionId functionId,
+                object entityForMessage,
+                EventLevel eventLevel,
+                CancellationToken cancellationToken)
             {
-                return EmptyLogBlock.Instance;
+                var blockId = GetNextUniqueBlockId();
+                var logBlock = new LogBlock(functionId, message: null, entityForMessage, eventLevel, blockId, cancellationToken);
+                logBlock.OnStart();
+                return logBlock;
             }
 
-            return CreateLogBlock(functionId, message, cancellationToken);
+            public static LogBlock Create(
+                FunctionId functionId,
+                string message,
+                EventLevel eventLevel,
+                CancellationToken cancellationToken)
+            {
+                var blockId = GetNextUniqueBlockId();
+                var logBlock = new LogBlock(functionId, message, entityForMessage: null, eventLevel, blockId, cancellationToken);
+                logBlock.OnStart();
+                return logBlock;
             }
 
             /// <summary>
@@ -47,44 +105,46 @@ private static int GetNextUniqueBlockId()
                 return Interlocked.Increment(ref s_lastUniqueBlockId);
             }
 
-        private static IDisposable CreateLogBlock(FunctionId functionId, string message, CancellationToken cancellationToken)
+            private void OnStart()
             {
-            var block = s_pool.Allocate();
-            var blockId = GetNextUniqueBlockId();
-            block.Construct(functionId, message, blockId, cancellationToken);
-            return block;
+                if (EnsureMessageIfLoggingEnabled())
+                {
+                    Debug.Assert(_message != null);
+                    Debug.Assert(!_startLogged);
+
+                    Instance.BlockStart(_message, _functionId, _blockId);
+                    _startLogged = true;
+                }
             }
 
-        /// <summary>
-        /// This tracks the logged message. On instantiation, it logs 'Started block' with other event data.
-        /// On dispose, it logs 'Ended block' with the same event data so we can track which block started and ended when looking at logs.
-        /// </summary>
-        private class RoslynLogBlock : IDisposable
+            private bool EnsureMessageIfLoggingEnabled()
+            {
+                if (Instance.IsEnabled(_eventLevel, EventKeywords.None))
                 {
-            private readonly ObjectPool<RoslynLogBlock> _pool;
-            private CancellationToken _cancellationToken;
+                    _message ??= (_entityForMessage?.ToString() ?? string.Empty);
+                    return true;
+                }
 
-            private FunctionId _functionId;
-            private int _tick;
-            private int _blockId;
+                return false;
+            }
 
-            public RoslynLogBlock(ObjectPool<RoslynLogBlock> pool)
+            public void Dispose()
+            {
+                if (!EnsureMessageIfLoggingEnabled())
                 {
-                _pool = pool;
+                    return;
                 }
 
-            public void Construct(FunctionId functionId, string message, int blockId, CancellationToken cancellationToken)
+                if (!_startLogged)
                 {
-                _functionId = functionId;
-                _tick = Environment.TickCount;
-                _blockId = blockId;
-                _cancellationToken = cancellationToken;
-
-                Instance.BlockStart(message, functionId, blockId);
+                    // User enabled logging after the block start.
+                    // We log a block start to log the message along with the block ID.
+                    Instance.BlockStart(_message, _functionId, _blockId);
+                    _startLogged = true;
                 }
 
-            public void Dispose()
-            {
+                Debug.Assert(_message != null);
+
                 // This delta is valid for durations of < 25 days
                 var delta = Environment.TickCount - _tick;
 
@@ -96,9 +156,6 @@ public void Dispose()
                 {
                     Instance.BlockStop(_functionId, delta, _blockId);
                 }
-
-                // Free this block back to the pool
-                _pool.Free(this);
             }
         }
     }