Default out-of-proc node pipe buffers to 1 MB (change wave 18.9) - #14094
Conversation
Out-of-process .NET node and TaskHost communication uses a named pipe whose kernel buffer was fixed at 128 KB. With synchronous, backpressure-bound writes, shipping large TaskHostConfiguration packets (avg ~46 KB, up to ~1.5 MB) to sidecar TaskHosts in -mt builds blocks the sender until the child drains the buffer. A 1 MB buffer pipelines many packets so the sender rarely blocks; on an OrchardCore.Cms.Web -mt rebuild this cut the measured parent->child transport time from ~114 s to ~9 s (~12x). The buffer size now lives in Traits (NodeConnectionBufferSize), so it is resettable for tests and tunable via MSBUILDNODECONNECTIONBUFFERSIZE. When unset it is 1 MB under change wave 18.9 and falls back to the historical 128 KB when the wave is opted out (MSBUILDDISABLEFEATURESFROMVERSION=18.9). The legacy .NET Framework 3.5 task host uses its own endpoint (src/MSBuildTaskHost) and is intentionally unaffected; it keeps 128 KB. Buffer sizes are per-stream OS hints that are never negotiated, and the parent always connects as a client without specifying a buffer, so a new parent and an old 128 KB child interoperate unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves MSBuild -mt throughput by increasing the default named-pipe kernel buffer size used for out-of-proc .NET nodes/TaskHosts, while gating the resource change behind change wave 18.9 and providing an environment override.
Changes:
- Replace the hardcoded 128 KB pipe buffer constant with a
Traits-backed setting, honoring change waves andMSBUILDNODECONNECTIONBUFFERSIZE. - Add
Traits.NodeConnectionBufferSizewith wave-dependent defaults (1 MB in wave 18.9; 128 KB when opted out). - Add unit tests covering the default/opt-out/override behavior and document the wave feature in
ChangeWaves.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Shared/NodeEndpointOutOfProcBase.cs | Switches pipe buffer sizing from a constant to Traits.Instance.NodeConnectionBufferSize. |
| src/Framework/Traits.cs | Introduces NodeConnectionBufferSize trait with change-wave defaulting and env-var override. |
| src/Build.UnitTests/ChangeWaves_Tests.cs | Adds tests validating buffer-size behavior across wave enablement and env override. |
| documentation/wiki/ChangeWaves.md | Documents the 18.9 wave item for the new default pipe buffer size + override. |
|
/review |
|
D4 review complete. Two issues found; reporting below. |
There was a problem hiding this comment.
D2 — ChangeWave Discipline: LGTM
All four checkpoints pass with no concrete failing scenario found.
1. Behavioral change correctly gated
GetNodeConnectionBufferSize() in Traits.cs calls ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_9) before returning the 1 MB value; when the wave is opted out it falls through to the legacy 128 KB constant. The pipe-buffer size is consumed via private static int PipeBufferSize => Traits.Instance.NodeConnectionBufferSize, so the gate is always evaluated.
2. Wave18_9 is the current upcoming wave
ChangeWaves.cs (PR head) declares AllWaves = [Wave17_10, ..., Wave18_8, Wave18_9]; HighestWave resolves to Wave18_9, confirming it is the active incoming wave.
3. Both paths tested
NodeConnectionBufferSizeIsLargeWhenWaveEnabled— wave enabled → 1 MB ✓NodeConnectionBufferSizeStaysLegacyWhenWaveDisabled— wave disabled → 128 KB ✓NodeConnectionBufferSizeRespectsEnvironmentOverride— env var overrides wave → 256 KB ✓
4. Documented in ChangeWaves.md
Entry added under ### 18.9 with opt-out instructions and a link to the PR. ✓
Generated by Expert Code Review (command) for issue #14094 · 1.7K AIC · ⊞ 29.8K ambient context
Comment /review to run again
AR-May
left a comment
There was a problem hiding this comment.
Great finding, the pr looks good to me. I have only one question for the team to discuss.
|
Still gathering evidence how much Taskhost overhead remains. traces indicate that while this is helpful it's not the complete reason for slowness |
…ize constant names, add pipe-creation trace, note test scope Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review feedback addressed in 5cb8f45
All 3 change-wave tests pass on net10.0 and net472 (6/6). The temporary file-dump instrumentation used for the measurement update above was kept out of this branch. |
Measurement update — honest end-to-end numbers (single Windows dev box)Following review discussion, I instrumented the task-host round trip directly (dispatch time in the parent, execute time in each sidecar host; Results
Why a ~12× transport ratio collapses to ~5%Two effects, both measured:
Relationship to the shared-memory prototype (#14054)The MMF channel did not reduce (de)serialization — it serializes/deserializes identically and adds two memcpys. It looked faster for the same reason as the 1 MB buffer: it decouples the parent's send call from the child's deserialize (dump bytes + signal, return). That's why the pipe-buffer sweep found 1 MB ≈ the single-slot MMF, and why this simple, cross-platform buffer bump supersedes the Windows-only MMF. Bottom lineThe win is real, cheap (~50 MB pool across ~25 hosts), and lands where round-trips are most serialized — but it is ~4–5% of task-host round-trip overhead / ~1–2% wall on this box, not a 10× build-time win. The remaining lever is (de)serialization / re-sending near-identical configs, which no transport change addresses. (Note: numbers above are from a single Windows dev machine and differ from the earlier PerfView analysis, which ran on different hardware and measured a different change — tasks moving in-proc — so the two are orthogonal.) |
|
Have we considered async io instead of blocking? |
Summary
Out-of-process .NET node and TaskHost communication uses a named pipe whose kernel buffer has been fixed at 128 KB. Because the sender writes packets synchronously, a named-pipe write blocks until the receiver drains the buffer when the buffer is full. In multi-threaded (
-mt) builds, non-thread-safe tasks (notably Csc/Vbc) run in sidecar TaskHosts, and theirTaskHostConfigurationpackets are large (avg ~46 KB, up to ~1.5 MB). With a 128 KB buffer the sender spends almost all its time blocked on backpressure.This change makes the buffer size a Traits setting (
NodeConnectionBufferSize) — so it is resettable for tests and tunable viaMSBUILDNODECONNECTIONBUFFERSIZE— and defaults it to 1 MB under change wave 18.9, falling back to the historical 128 KB when the wave is opted out.Why 1 MB (data)
Sweeping the pipe buffer with no other changes (3 runs each,
OrchardCore.Cms.Web -t:Rebuild -mt, parent→child transport time, median):Send time roughly halves per doubling up to ~1 MB, then plateaus. 1 MB gives ~12× on the send path for a modest, bounded memory cost. A 1 MB buffer is a deep enough queue (~11 average configs, or one whole large config) that the sender dumps-and-moves-on instead of blocking per packet. Bigger buffers add non-paged kernel pool for no further benefit.
Why a change wave
This is a behavioral/resource change (more non-paged kernel pool per node — ~50 MB at a measured peak of 25 concurrent TaskHosts) rather than a pure bug fix, so it ships behind change wave 18.9 with an opt-out (
MSBUILDDISABLEFEATURESFROMVERSION=18.9) per repo policy.Compatibility / legacy task host
src/MSBuildTaskHost) uses its own endpoint with its own unchanged 128 KB constant, so it is intentionally unaffected and keeps 128 KB.Tests
ChangeWaves_Testsadds coverage thatTraits.NodeConnectionBufferSizeis:MSBUILDNODECONNECTIONBUFFERSIZEoverride regardless of the wave.All pass on net10.0 and net472. Repo builds clean (0 warnings / 0 errors); an end-to-end
-mtTaskHost build succeeds with the new default.Notes
This is the focused, product-ready slice of a broader IPC investigation (the larger shared-memory transport prototype, with full per-task measurements and a Unix analysis, is tracked separately in #14054). The pipe-buffer bump is the cheapest, lowest-risk, cross-platform win and is independent of that prototype.