fb

Author: BrennanConroy

src/Servers/HttpSys/src/HttpSysListener.cs

@@ -3,7 +3,6 @@
 
 using System.Buffers;
 using System.Diagnostics;
-using System.Diagnostics.Metrics;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.HttpSys.Internal;
@@ -35,13 +34,6 @@ internal sealed partial class HttpSysListener : IDisposable
     // 0.5 seconds per request.  Respond with a 400 Bad Request.
     private const int UnknownHeaderLimit = 1000;
 
-    internal sealed class NoopMeterFactory : IMeterFactory
-    {
-        public Meter Create(MeterOptions options) => new Meter(options);
-
-        public void Dispose() { }
-    }
-
     internal MemoryPool<byte> MemoryPool { get; }
 
     private volatile State _state; // m_State is set only within lock blocks, but often read outside locks.

src/Shared/Buffers.MemoryPool/DefaultMemoryPoolFactory.cs

@@ -6,21 +6,22 @@
 using System.Diagnostics;
 using System.Diagnostics.Metrics;
 using Microsoft.AspNetCore.Connections;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore;
 
 #nullable enable
 
 internal sealed class DefaultMemoryPoolFactory : IMemoryPoolFactory<byte>, IAsyncDisposable
 {
-    private readonly IMeterFactory _meterFactory;
-    private readonly ConcurrentDictionary<PinnedBlockMemoryPool, PinnedBlockMemoryPool> _pools = new();
+    private readonly IMeterFactory? _meterFactory;
+    private readonly ConcurrentDictionary<PinnedBlockMemoryPool, bool> _pools = new();
     private readonly PeriodicTimer _timer;
     private readonly Task _timerTask;
 
-    public DefaultMemoryPoolFactory(IMeterFactory? meterFactory = null)
+    public DefaultMemoryPoolFactory(IMeterFactory? meterFactory = null, ILogger<DefaultMemoryPoolFactory>? logger = null)
     {
-        _meterFactory = meterFactory ?? NoopMeterFactory.Instance;
+        _meterFactory = meterFactory;
         _timer = new PeriodicTimer(TimeSpan.FromSeconds(10));
         _timerTask = Task.Run(async () =>
         {
@@ -36,7 +37,7 @@ public DefaultMemoryPoolFactory(IMeterFactory? meterFactory = null)
             }
             catch (Exception ex)
             {
-                Debug.WriteLine(ex);
+                logger?.LogCritical(ex, "Error while evicting memory from pools.");
             }
         });
     }
@@ -45,11 +46,11 @@ public MemoryPool<byte> Create()
     {
         var pool = new PinnedBlockMemoryPool(_meterFactory);
 
-        _pools.TryAdd(pool, pool);
+        _pools.TryAdd(pool, true);
 
         pool.OnPoolDisposed(static (state, self) =>
         {
-            ((ConcurrentDictionary<PinnedBlockMemoryPool, PinnedBlockMemoryPool>)state!).TryRemove(self, out _);
+            ((ConcurrentDictionary<PinnedBlockMemoryPool, bool>)state!).TryRemove(self, out _);
         }, _pools);
 
         return pool;
@@ -60,15 +61,4 @@ public async ValueTask DisposeAsync()
         _timer.Dispose();
         await _timerTask;
     }
-
-    private sealed class NoopMeterFactory : IMeterFactory
-    {
-        public static NoopMeterFactory Instance = new NoopMeterFactory();
-
-        public Meter Create(MeterOptions options) => new Meter(options);
-
-        public void Dispose()
-        {
-        }
-    }
 }

src/Shared/Buffers.MemoryPool/MemoryPoolFactory.cs

@@ -12,7 +12,6 @@ internal static class TestMemoryPoolFactory
 {
     public static MemoryPool<byte> Create(IMeterFactory? meterFactory = null)
     {
-        meterFactory ??= NoopMeterFactory.Instance;
 #if DEBUG
         return new DiagnosticMemoryPool(CreatePinnedBlockMemoryPool(meterFactory));
 #else
@@ -22,18 +21,6 @@ public static MemoryPool<byte> Create(IMeterFactory? meterFactory = null)
 
     public static MemoryPool<byte> CreatePinnedBlockMemoryPool(IMeterFactory? meterFactory = null)
     {
-        meterFactory ??= NoopMeterFactory.Instance;
         return new PinnedBlockMemoryPool(meterFactory);
     }
-
-    private sealed class NoopMeterFactory : IMeterFactory
-    {
-        public static NoopMeterFactory Instance = new NoopMeterFactory();
-
-        public Meter Create(MeterOptions options) => new Meter(options);
-
-        public void Dispose()
-        {
-        }
-    }
 }

src/Shared/Buffers.MemoryPool/PinnedBlockMemoryPool.cs

@@ -41,7 +41,7 @@ internal sealed class PinnedBlockMemoryPool : MemoryPool<byte>, IThreadPoolWorkI
     /// </summary>
     private bool _isDisposed; // To detect redundant calls
 
-    private readonly PinnedBlockMemoryPoolMetrics _metrics;
+    private readonly PinnedBlockMemoryPoolMetrics? _metrics;
 
     private long _currentMemory;
     private long _evictedMemory;
@@ -60,14 +60,9 @@ internal sealed class PinnedBlockMemoryPool : MemoryPool<byte>, IThreadPoolWorkI
     /// </summary>
     private const int AnySize = -1;
 
-    public PinnedBlockMemoryPool()
-        : this(NoopMeterFactory.Instance)
+    public PinnedBlockMemoryPool(IMeterFactory? meterFactory = null)
     {
-    }
-
-    public PinnedBlockMemoryPool(IMeterFactory meterFactory)
-    {
-        _metrics = new(meterFactory);
+        _metrics = meterFactory is null ? null : new PinnedBlockMemoryPoolMetrics(meterFactory);
     }
 
     /// <summary>
@@ -95,16 +90,16 @@ public override IMemoryOwner<byte> Rent(int size = AnySize)
 
         if (_blocks.TryDequeue(out var block))
         {
-            _metrics.UpdateCurrentMemory(-block.Memory.Length);
-            _metrics.Rent(block.Memory.Length);
+            _metrics?.UpdateCurrentMemory(-block.Memory.Length);
+            _metrics?.Rent(block.Memory.Length);
             Interlocked.Add(ref _currentMemory, -block.Memory.Length);
 
             // block successfully taken from the stack - return it
             return block;
         }
 
-        _metrics.IncrementTotalMemory(BlockSize);
-        _metrics.Rent(BlockSize);
+        _metrics?.IncrementTotalMemory(BlockSize);
+        _metrics?.Rent(BlockSize);
 
         // We already counted this Rent call above, but since we're now allocating (need more blocks)
         // that means the pool is 'very' active and we probably shouldn't evict blocks, so we count again
@@ -134,7 +129,7 @@ internal void Return(MemoryPoolBlock block)
 
         if (!_isDisposed)
         {
-            _metrics.UpdateCurrentMemory(block.Memory.Length);
+            _metrics?.UpdateCurrentMemory(block.Memory.Length);
             Interlocked.Add(ref _currentMemory, block.Memory.Length);
 
             _blocks.Enqueue(block);
@@ -201,8 +196,8 @@ internal void PerformEviction()
         // Remove from queue and let GC clean the memory up
         while (burstAmount > 0 && _blocks.TryDequeue(out var block))
         {
-            _metrics.UpdateCurrentMemory(-block.Memory.Length);
-            _metrics.EvictBlock(block.Memory.Length);
+            _metrics?.UpdateCurrentMemory(-block.Memory.Length);
+            _metrics?.EvictBlock(block.Memory.Length);
             Interlocked.Add(ref _currentMemory, -block.Memory.Length);
             Interlocked.Add(ref _evictedMemory, block.Memory.Length);
 
@@ -236,15 +231,4 @@ protected override void Dispose(bool disposing)
 
     // Used for testing
     public int BlockCount() => _blocks.Count;
-
-    private sealed class NoopMeterFactory : IMeterFactory
-    {
-        public static NoopMeterFactory Instance { get; } = new();
-
-        public Meter Create(MeterOptions options) => new Meter(options);
-
-        public void Dispose()
-        {
-        }
-    }
 }