Skip to content

Bound the pooled queues by the number of items they contained - #1443

Merged
meziantou merged 1 commit into
mainfrom
feature/queue-pooling-memory-bound-dbf04a
Sep 8, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/queue-pooling-memory-bound-dbf04a

Conversation

@meziantou

Copy link
Copy Markdown
Owner

What

QueuePooledObjectPolicy<T>.Return discarded a queue when its Count was greater than MaximumRetainedCount, so it never discarded anything: the consumers dequeue all the items before returning the queue to the pool, so Count is 0 whatever the number of items the queue contained. As Queue<T>.Clear does not release the backing array, the static pool retained the array of a queue that grew a lot, such as the one UseLangwordInXmlCommentAnalyzer uses to walk an unusually wide documentation comment. The items themselves were released by Clear, so what was retained was the array storage, not the syntax nodes.

PooledQueue<T> now wraps the Queue<T> and keeps track of the maximum number of items it contained since it was last cleared, and the policy uses that high-water mark as its bound. MaximumRetainedCount keeps its default of 1024, and is now an approximation of the capacity of the backing array, which is within a factor of about 2 of the peak count.

Why not the capacity of the queue

The assembly loaded by Roslyn is the netstandard2.0 one, where Queue<T>.Capacity does not exist: it was added in .NET 9. A check guarded by #if NET9_0_OR_GREATER would only have bounded the net10.0 build used by the tests, and not the analyzer that ships. Tracking the high-water mark behaves the same way for all the target frameworks and needs no reflection.

Notes for the reviewer

  • UseLangwordInXmlCommentAnalyzer only changes the type of its pool field; the traversal itself is untouched.
  • PooledQueue<T> exposes only what the consumers and the policy need (Count, MaximumCount, Enqueue, TryDequeue, Clear). It is not enumerable, so one existing assertion moved from Assert.Empty to Assert.Equal(0, queue.Count).

Tests

Two tests cover the grow, drain and return path the previous bound missed, one on the policy and one on the pool. Reverting only the MaximumCount comparison back to Count makes exactly those two fail.

  • dotnet build: succeeded for the five Roslyn versions, 0 warnings.
  • dotnet test --filter "FullyQualifiedName~ObjectPoolTests|FullyQualifiedName~UseLangwordInXmlComment": 160 passed, 0 failed (32 tests for each of the 5 Roslyn versions).
  • dotnet run --project src/DocumentationGenerator: exit code 0, no markdown change.

QueuePooledObjectPolicy<T>.Return discarded a queue when its Count was
greater than MaximumRetainedCount, but the consumers dequeue all the items
before returning the queue to the pool, so Count was 0 whatever the number
of items the queue contained. As Queue<T>.Clear does not release the
backing array, the pool retained the array of a queue that grew a lot, such
as the one used by UseLangwordInXmlCommentAnalyzer to walk an unusually wide
documentation comment.

The capacity of the queue cannot be used to detect these queues: the
assembly loaded by Roslyn is the netstandard2.0 one, and Queue<T>.Capacity
is only available from .NET 9, so the check would only apply to the build
used by the tests. Introduce PooledQueue<T>, which keeps track of the
maximum number of items it contained since it was last cleared, and use it
as the bound in the policy so the behavior is the same for all the target
frameworks.
@meziantou
meziantou merged commit 2e4ab65 into main Sep 8, 2026
13 checks passed
@meziantou
meziantou deleted the feature/queue-pooling-memory-bound-dbf04a branch September 8, 2026 17:34
This was referenced Sep 8, 2026
This was referenced Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant