diff --git a/src/Meziantou.Analyzer/Internals/ObjectPool.cs b/src/Meziantou.Analyzer/Internals/ObjectPool.cs index 336760289..f14096869 100644 --- a/src/Meziantou.Analyzer/Internals/ObjectPool.cs +++ b/src/Meziantou.Analyzer/Internals/ObjectPool.cs @@ -1,6 +1,7 @@ #pragma warning disable MA0048 // File name must match type name #pragma warning disable RS1035 // Do not use APIs banned for analyzers using System.Collections.Concurrent; +using System.Diagnostics.CodeAnalysis; namespace Meziantou.Analyzer.Internals; @@ -42,7 +43,7 @@ public static ObjectPool CreateStringBuilderPool() return provider.Create(new StringBuilderPooledObjectPolicy()); } - public static ObjectPool> CreateQueuePool() + public static ObjectPool> CreateQueuePool() { var provider = new DefaultObjectPoolProvider(); return provider.Create(new QueuePooledObjectPolicy()); @@ -354,30 +355,72 @@ public override bool Return(StringBuilder obj) } /// -/// A policy for pooling instances. +/// A queue that keeps track of the maximum number of items it contained, so a pool can detect the instances whose +/// backing array grew too much. cannot be used for that purpose because the consumers +/// usually dequeue all the items before returning the queue to the pool, and does not +/// release the backing array. +/// +/// The type of the items of the queue. +internal sealed class PooledQueue +{ + private readonly Queue _queue = new(); + + /// + /// Gets the number of items contained in the queue. + /// + public int Count => _queue.Count; + + /// + /// Gets the maximum number of items the queue contained since the last call to . + /// + public int MaximumCount { get; private set; } + + public void Enqueue(T item) + { + _queue.Enqueue(item); + if (_queue.Count > MaximumCount) + { + MaximumCount = _queue.Count; + } + } + + public bool TryDequeue([MaybeNullWhen(false)] out T result) + { + return _queue.TryDequeue(out result); + } + + public void Clear() + { + _queue.Clear(); + MaximumCount = 0; + } +} + +/// +/// A policy for pooling instances. /// /// The type of the items of the pooled queues. -internal sealed class QueuePooledObjectPolicy : PooledObjectPolicy> +internal sealed class QueuePooledObjectPolicy : PooledObjectPolicy> { /// - /// Gets or sets the maximum number of items a can contain to be retained, - /// when is invoked. + /// Gets or sets the maximum number of items a can have contained to be retained, + /// when is invoked. /// /// Defaults to 1024. public int MaximumRetainedCount { get; set; } = 1024; /// - public override Queue Create() + public override PooledQueue Create() { - return new Queue(); + return new PooledQueue(); } /// - public override bool Return(Queue obj) + public override bool Return(PooledQueue obj) { - if (obj.Count > MaximumRetainedCount) + if (obj.MaximumCount > MaximumRetainedCount) { - // Too big. Discard this one. + // The backing array grew too much. Discard this one. return false; } diff --git a/src/Meziantou.Analyzer/Rules/UseLangwordInXmlCommentAnalyzer.cs b/src/Meziantou.Analyzer/Rules/UseLangwordInXmlCommentAnalyzer.cs index 4cb5bc74b..462dd5176 100644 --- a/src/Meziantou.Analyzer/Rules/UseLangwordInXmlCommentAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/UseLangwordInXmlCommentAnalyzer.cs @@ -6,7 +6,7 @@ namespace Meziantou.Analyzer.Rules; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class UseLangwordInXmlCommentAnalyzer : DiagnosticAnalyzer { - private static readonly ObjectPool> NodeQueuePool = ObjectPool.CreateQueuePool(); + private static readonly ObjectPool> NodeQueuePool = ObjectPool.CreateQueuePool(); private static readonly HashSet CSharpKeywords = new(StringComparer.Ordinal) { diff --git a/tests/Meziantou.Analyzer.Test/Internals/ObjectPoolTests.cs b/tests/Meziantou.Analyzer.Test/Internals/ObjectPoolTests.cs index 0c84e07bb..4341f8b44 100644 --- a/tests/Meziantou.Analyzer.Test/Internals/ObjectPoolTests.cs +++ b/tests/Meziantou.Analyzer.Test/Internals/ObjectPoolTests.cs @@ -12,7 +12,7 @@ public void QueuePooledObjectPolicy_ClearsTheQueueOnReturn() queue.Enqueue(new object()); Assert.True(policy.Return(queue)); - Assert.Empty(queue); + Assert.Equal(0, queue.Count); } [Fact] @@ -26,6 +26,33 @@ public void QueuePooledObjectPolicy_DoesNotRetainOversizedQueues() Assert.False(policy.Return(queue)); } + [Fact] + public void QueuePooledObjectPolicy_DoesNotRetainOversizedQueuesThatWereDrained() + { + var policy = new QueuePooledObjectPolicy { MaximumRetainedCount = 1 }; + var queue = policy.Create(); + queue.Enqueue(new object()); + queue.Enqueue(new object()); + while (queue.TryDequeue(out _)) + { + } + + Assert.Equal(0, queue.Count); + Assert.False(policy.Return(queue)); + } + + [Fact] + public void QueuePooledObjectPolicy_ResetsTheMaximumCountOnReturn() + { + var policy = new QueuePooledObjectPolicy { MaximumRetainedCount = 1 }; + var queue = policy.Create(); + queue.Enqueue(new object()); + + Assert.True(policy.Return(queue)); + Assert.Equal(0, queue.MaximumCount); + Assert.True(policy.Return(queue)); + } + [Fact] public void QueuePool_DoesNotKeepTheItemsAlive() { @@ -34,6 +61,22 @@ public void QueuePool_DoesNotKeepTheItemsAlive() queue.Enqueue(new object()); pool.Return(queue); - Assert.Empty(pool.Get()); + Assert.Equal(0, pool.Get().Count); + } + + [Fact] + public void QueuePool_DoesNotReuseADrainedOversizedQueue() + { + var pool = ObjectPool.Create(new QueuePooledObjectPolicy { MaximumRetainedCount = 1 }); + var queue = pool.Get(); + queue.Enqueue(new object()); + queue.Enqueue(new object()); + while (queue.TryDequeue(out _)) + { + } + + pool.Return(queue); + + Assert.NotSame(queue, pool.Get()); } }