Reduce reader/writer contention in Pipelines

[VSadov]

The change deals with excessive contentions coming from Pipelines in some benchmarks when reader and writer contend for the lock used to protect shared state of the Pipe.

Changes:

• move some operations that may take nontrivial time while not requiring locking outside of the lock. That is mostly renting/returning byte buffers. That could end up in allocations or coordinating with other threads accessing the pool, but by itself does not need to lock the Pipe.
• make the internal per-Pipe pool of segment objects a FIFO, so that writer (which returns segments) and reader (which takes them out) operate at different ends of the pool - to reduce cache line sharing in rent/return operations.
• switch the Pipe lock from monitor lock to Threading.Lock. This lock often inflates anyways.
• schedule continuations to local ThreadPool queues (vs. global) - to keep logical "pipelining" running on the same physical thread, if possible.
  For example, writer thread, if it looks for more work after writing, would be preferred to take care of asynchronous reading and processing of written data, thus further reducing contention and improving locality of access.

=== effect on JSON asp.net benchmark running on a 56-core machine:

• Contentions:

- Max Lock Contention (#/s)               | 483
+ Max Lock Contention (#/s)               | 66

• RPS:

- Requests/sec              | 2,094,374
+ Requests/sec              | 2,148,465

• Throughput:

- Read throughput (MB/s)    | 291.61
+ Read throughput (MB/s)    | 299.14

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[VSadov]

=== typical counts before the PR:

| application                             |                                         |
| --------------------------------------- | --------------------------------------- |
| Max Process CPU Usage (%)               | 87                                      |
| Max Cores usage (%)                     | 4,892                                   |
| Max Working Set (MB)                    | 95                                      |
| Max Private Memory (MB)                 | 710                                     |
| Build Time (ms)                         | 6,801                                   |
| Start Time (ms)                         | 218                                     |
| Published Size (KB)                     | 111,948                                 |
| Symbols Size (KB)                       | 55                                      |
| .NET Core SDK Version                   | 11.0.100-preview.7.26365.114            |
| ASP.NET Core Version                    | 11.0.0-preview.7.26365.114+8c1bd18b8c65 |
| .NET Runtime Version                    | 11.0.0-preview.7.26365.114+8c1bd18b8c65 |
| Max Global CPU Usage (%)                | 92                                      |
| Max CPU Usage (%)                       | 90                                      |
| Max Working Set (MB)                    | 99                                      |
| Max GC Heap Size (MB)                   | 14                                      |
| Size of committed memory by the GC (MB) | 11                                      |
| Max Number of Gen 0 GCs / sec           | 2.00                                    |
| Max Number of Gen 1 GCs / sec           | 1.00                                    |
| Max Number of Gen 2 GCs / sec           | 1.00                                    |
| Max Gen 0 GC Budget (MB)                | 19                                      |
| Max Time in GC (%)                      | 13.00                                   |
| Max Gen 0 Size (B)                      | 0                                       |
| Max Gen 1 Size (B)                      | 1,530,184                               |
| Max Gen 2 Size (B)                      | 1,704,952                               |
| Max LOH Size (B)                        | 0                                       |
| Max POH Size (B)                        | 3,147,488                               |
| Max Allocation Rate (B/sec)             | 7,875,256                               |
| Max GC Heap Fragmentation (%)           | 569%                                    |
| # of Assemblies Loaded                  | 63                                      |
| Max Exceptions (#/s)                    | 896                                     |
| Max Lock Contention (#/s)               | 483                                     |
| Max ThreadPool Threads Count            | 63                                      |
| Max ThreadPool Queue Length             | 131                                     |
| Max ThreadPool Items (#/s)              | 2,510,219                               |
| Max Active Timers                       | 1                                       |
| IL Jitted (B)                           | 231,701                                 |
| Methods Jitted                          | 2,560                                   |


| load                      |            |
| ------------------------- | ---------- |
| Max Process CPU Usage (%) | 47         |
| Max Cores usage (%)       | 2,618      |
| Max Working Set (MB)      | 47         |
| Max Private Memory (MB)   | 363        |
| Start Time (ms)           | 1          |
| Max Global CPU Usage (%)  | 60         |
| First Request (ms)        | 131        |
| Requests/sec              | 2,094,374  |
| Requests                  | 31,623,276 |
| Mean latency (ms)         | 0.24       |
| Max latency (ms)          | 9.13       |
| Bad responses             | 0          |
| Socket errors             | 0          |
| Read throughput (MB/s)    | 291.61     |
| Latency 50th (ms)         | 0.22       |
| Latency 75th (ms)         | 0.28       |
| Latency 90th (ms)         | 0.37       |
| Latency 99th (ms)         | 0.57       |

[VSadov]

=== typical counts after the PR:

| application                             |                                         |
| --------------------------------------- | --------------------------------------- |
| Max Process CPU Usage (%)               | 89                                      |
| Max Cores usage (%)                     | 4,969                                   |
| Max Working Set (MB)                    | 94                                      |
| Max Private Memory (MB)                 | 702                                     |
| Build Time (ms)                         | 8,201                                   |
| Start Time (ms)                         | 207                                     |
| Published Size (KB)                     | 111,948                                 |
| Symbols Size (KB)                       | 55                                      |
| .NET Core SDK Version                   | 11.0.100-preview.7.26365.114            |
| ASP.NET Core Version                    | 11.0.0-preview.7.26365.114+8c1bd18b8c65 |
| .NET Runtime Version                    | 11.0.0-preview.7.26365.114+8c1bd18b8c65 |
| Max Global CPU Usage (%)                | 88                                      |
| Max CPU Usage (%)                       | 85                                      |
| Max Working Set (MB)                    | 98                                      |
| Max GC Heap Size (MB)                   | 15                                      |
| Size of committed memory by the GC (MB) | 12                                      |
| Max Number of Gen 0 GCs / sec           | 3.00                                    |
| Max Number of Gen 1 GCs / sec           | 2.00                                    |
| Max Number of Gen 2 GCs / sec           | 1.00                                    |
| Max Gen 0 GC Budget (MB)                | 36                                      |
| Max Time in GC (%)                      | 85.00                                   |
| Max Gen 0 Size (B)                      | 0                                       |
| Max Gen 1 Size (B)                      | 20,368                                  |
| Max Gen 2 Size (B)                      | 4,378,240                               |
| Max LOH Size (B)                        | 0                                       |
| Max POH Size (B)                        | 3,147,488                               |
| Max Allocation Rate (B/sec)             | 8,455,664                               |
| Max GC Heap Fragmentation (%)           | 717%                                    |
| # of Assemblies Loaded                  | 63                                      |
| Max Exceptions (#/s)                    | 916                                     |
| Max Lock Contention (#/s)               | 66                                      |
| Max ThreadPool Threads Count            | 62                                      |
| Max ThreadPool Queue Length             | 223                                     |
| Max ThreadPool Items (#/s)              | 2,881,024                               |
| Max Active Timers                       | 1                                       |
| IL Jitted (B)                           | 240,531                                 |
| Methods Jitted                          | 2,791                                   |


| load                      |            |
| ------------------------- | ---------- |
| Max Process CPU Usage (%) | 48         |
| Max Cores usage (%)       | 2,669      |
| Max Working Set (MB)      | 47         |
| Max Private Memory (MB)   | 363        |
| Start Time (ms)           | 1          |
| Max Global CPU Usage (%)  | 61         |
| First Request (ms)        | 141        |
| Requests/sec              | 2,148,465  |
| Requests                  | 32,441,366 |
| Mean latency (ms)         | 0.23       |
| Max latency (ms)          | 5.70       |
| Bad responses             | 0          |
| Socket errors             | 0          |
| Read throughput (MB/s)    | 299.14     |
| Latency 50th (ms)         | 0.21       |
| Latency 75th (ms)         | 0.28       |
| Latency 90th (ms)         | 0.37       |
| Latency 99th (ms)         | 0.56       |

[Copilot]

Pull request overview

This PR updates System.IO.Pipelines internals to reduce reader/writer lock contention by changing how BufferSegment instances and backing memory are acquired/returned, and adjusts related scheduling and tests.

Changes:

• Switch Pipe’s segment pooling from a stack to an SPSC queue and return consumed segments outside the main lock.
• Speculatively rent backing memory outside the lock and add a lock-free fast path for Advance when writing is active.
• Update the segment-pool reuse test expectation (FIFO) and set ThreadPool scheduling to prefer local queues.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/libraries/System.IO.Pipelines/tests/BufferSegmentPoolTest.cs	Updates pooling test to expect FIFO reuse order.
src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/ThreadPoolScheduler.netcoreapp.cs	Uses preferLocal: true for ThreadPool scheduling.
src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeOptions.cs	Updates segment pool sizing defaults/comments (but currently misses required property assignment).
src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs	Moves to SPSC pooling + reduces time under the shared lock (memory rent + segment return).
src/libraries/System.IO.Pipelines/src/System.IO.Pipelines.csproj	Links in SPSC queue + padding shared sources (and introduces a BOM).

[VSadov]

Benchmark used (--scenario json --profile aspnet-gold-lin):

crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenarios/platform.benchmarks.yml --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/build/azure.profile.yml --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/build/azure.profile.yml --scenario json --profile aspnet-gold-lin --load.connections 512 --application.framework net11.0 --application.collectDependencies true --application.options.collectCounters true --load.options.reuseBuild true  --application.buildArguments "-p:Features=runtime-async=on"

The changes to notice:

• Contentions:

- Max Lock Contention (#/s)               | 483
+ Max Lock Contention (#/s)               | 66

• RPS:

- Requests/sec              | 2,094,374
+ Requests/sec              | 2,148,465

• Throughput:

- Read throughput (MB/s)    | 291.61
+ Read throughput (MB/s)    | 299.14

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[VSadov]

Build failures on NET481 seems unrelated to the change:

D:\a_work\1\s\src\libraries\Microsoft.Extensions.Caching.Abstractions\tests\TrimmingTests\HybridCacheEntryOptionsAccessorTest.cs(17,6): error CS0246: The type or namespace name 'UnsafeAccessorAttribute' could not be found (are you missing a using directive or an assembly reference?) [D:\a_work\1\s\src\libraries\Microsoft.Extensions.Caching.Abstractions\tests\Microsoft.Extensions.Caching.Abstractions.Tests.csproj::TargetFramework=net481]

#130892

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "640d9a6ed202c8c9d854bab61a3246db82999a81",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "4c9e771cbd5e35313cef06c4d7efb55070fd5493",
  "last_reviewed_commit": "640d9a6ed202c8c9d854bab61a3246db82999a81",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "4c9e771cbd5e35313cef06c4d7efb55070fd5493",
  "last_recorded_worker_run_id": "29685652058",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "640d9a6ed202c8c9d854bab61a3246db82999a81",
      "review_id": 4730708679
    }
  ]
}

[github-actions]

Holistic Review

Motivation: Under high core-count ASP.NET benchmarks the reader and writer of a Pipe contend heavily on the single lock that guards shared state, plus the FILO segment pool causes cache-line ping-pong because both threads touch the same end of the pool. The PR reports a large drop in max lock contention (483 -> 66 #/s) with modest RPS/throughput gains on a 56-core machine.

Approach: Four coordinated changes. (1) Move non-locking-required work out of the lock: memory rent/return and segment Reset()/return now happen outside SyncObj. AllocateWriteHeadSynchronized speculatively rents backing memory before taking the lock (RentMemoryUnsynchronized) and attaches it under the lock via AttachMemory, and AdvanceReader returns consumed segments in a finally after releasing the lock. (2) Replace the FILO BufferSegmentStack with a SingleProducerSingleConsumerQueue<BufferSegment> so the writer (dequeue/consumer) and reader (enqueue/producer) operate at opposite ends, reducing false sharing. (3) Switch SyncObj from a Monitor object to System.Threading.Lock on NET9+, with an object fallback for older TFMs. (4) Advance(int) takes a lock-free fast path while writing is active, and continuations are scheduled preferLocal: true to keep pipelining on the same physical thread.

Summary: The change is subtle but the concurrency reasoning is thorough and, as far as I can determine by inspection, correct. The SPSC role split is respected: GetOrCreateSegment (dequeue) is only reached from writer allocation paths, and ReturnSegment (enqueue) is only reached from the reader's AdvanceReader, consistent with the Pipe's single-reader/single-writer contract that SPSC requires. The off-lock _writingHeadMemory.Length reads are single-int reads (atomic, at worst stale), the authoritative decision is remade under the lock, and the Debug.Assert(prerented is null) documents/guards the "speculative rent always consumed" invariant. The lock-free Advance fast path is gated on IsWritingActive, where the writer thread exclusively owns the writing-head fields. InitialSegmentPoolSize is removed cleanly (internal-only, no ref-surface impact; StreamPipeReader/StreamPipeWriter keep their own constant and BufferSegmentStack, which is intentional). The MaxSegmentPoolSize documentation is a helpful addition. The one test change correctly flips the pool reuse assertion from FILO to FIFO ordering. I did not identify functional bugs or missing test coverage introduced by this PR; the existing pool/threshold tests continue to exercise the new pooling path. Verdict: LGTM. I recommend confirming the full CI matrix (including the netstandard2.0 / .NET Framework Monitor fallback path and stress/concurrency tests) is green before merge, since correctness here rests on threading invariants that are hard to cover exhaustively with unit tests.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 154.4 AIC · ⌖ 10.7 AIC · ⊞ 10K

[eduardo-vp]

LGTM! I was only surprised at the values of Max Time in GC (%) and Max Gen 2 Size (B) in the benchmark but I think the use of a more complex queue rather than a simpler stack is the reason we're seeing the increase in the Gen 2 - the numbers overall look great.

[VSadov]

/ba-g netfx failure is #130892

[VSadov]

LGTM! I was only surprised at the values of Max Time in GC (%) and Max Gen 2 Size (B) in the benchmark but I think the use of a more complex queue rather than a simpler stack is the reason we're seeing the increase in the Gen 2 - the numbers overall look great.

These benchmarks do not allocate much (1-3 gc per second in a multithreaded app running on 56 cores is fairly low), so this may be just sampling noise that depends on timing when those relatively few GCs happen. The end result does not seem to be impacted by that metric.