Skip to content

Client/transport: one recv per small chunk, ManagedSession lock fast path, write coalescing - #4737

Merged
romanett merged 6 commits into
masterfrom
romanett/client-perf-followups
Oct 12, 2026
Merged

romanett merged 6 commits into
masterfrom
romanett/client-perf-followups

Conversation

@romanett

@romanett romanett commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Follow-ups from the "Not changed" list of #4720 (client per-request allocation and CPU): three changes that the paired A/B shows a gain for, plus a write-up of the fourth (discarded ResponseHeader), which is not changed.

Results

Paired A/B, each change against the commit before it: 4 alternating rounds, medians, 20 sessions, 24-core machine shared with other sessions. Client = the harness process, server = the reference server child. Allocated bytes per operation are deterministic; throughput and latency carry the run-to-run noise noted below.

Change Scenario Throughput p50 p90 Client CPU / op Client alloc / op Server CPU / op
Receive staging Read, 1 value +4% −9% −13% ≈ ±0 ≈
Read, 1 value, SignAndEncrypt +6% −6% −11% ≈ ±0 ≈
Read, 100 values +2% −4% ≈ ≈ ±0 ≈
Data changes ≈ ≈ ≈ ≈ ±0 ≈
ManagedSession lock Read, 1 value, ManagedSession ≈ ≈ ≈ ≈ −125 B (2632 → 2507) ≈
Same, inflight 8 ≈ ≈ ≈ ≈ −133 B ≈
Write coalescing Read, 1 value, 20 sessions on one shared channel, inflight 4 +13% −13% −3% −6% ±0 −4%
Same, inflight 1 +20% ¹ −8% ¹ −9% ±0 −13%
Read, 1 value, inflight 8 (one channel per session) +2% −4% −6% ≈ ±0 ≈
Read, 100 values, inflight 8 +3% −4% −8% ≈ ±0 ≈
Data changes ≈ ≈ ≈ ≈ ±0 ≈

≈: within run-to-run noise (about ±5% for CPU per op here).
¹ Rounds ranged from −42% to +32%; the inflight-4 runs were better in all four rounds.

Changes

  • TcpByteTransport receive: ReceiveChunkAsync read the 8-byte chunk header and then the body, two recv calls per chunk. It now waits for data with a small staging buffer (8 KB, allocated on the first receive, not from the BufferManager budget). One receive returns the header and, when the chunk is small and already there, the whole chunk; bytes of following chunks are kept for the next call. A larger chunk is completed with a receive into the rented buffer, as before.
    • The receive buffer is still rented only after the header has been validated (message type, size against the negotiated limit), so an idle peer holds none of the budget.
    • A receive cancelled while its bytes are in the staging buffer keeps them for the next call; on .NET Framework the staging receive takes no token, so cancellation is observed once data arrives.
    • A receive that fails while reading the body of a chunk larger than the staging buffer has consumed part of the chunk; later receives then fail with BadTcpInternalError rather than read body bytes as a header. Before this PR such a failure lost the header and body bytes silently. The channel cancels receives only when it closes.
    • Server listener channels use the same transport.
  • ManagedSession service calls: each call was an async wrapper (await the reader lock, await the inner call), one more state machine box per request.
    • When services are available and no writer holds or waits for the lock, the reader lock is taken synchronously (AsyncReaderWriterLock.TryEnterReader). It is released by a cached continuation on the inner call's task, registered before the caller's await, so the lock is free again when the caller resumes.
    • Otherwise the call waits for the lock as before, so reconnect and failover still block new calls and drain the ones in flight.
    • The remaining 88 B per call is the task's continuation list (two continuations).
  • Write coalescing (UaSCUaBinaryChannel): when more writes for the same transport are queued behind the one being sent, the writer takes up to 16 of them and sends their segments in one vectored send.
    • Each write is then reported on its own, in order, with its size. A failed send is reported for every write in it, and their buffers are returned.
    • Writes still never start on the caller's stack, and one writer sends one batch at a time, so chunk order is unchanged.
    • Only byte-stream transports opt in (TcpByteTransport, also through the server's admission lease, via the internal IUaSCChunkCoalescing). WebSocket and capture decorators keep one send per chunk.

Behaviour notes

  • Each TCP connection that has received holds an 8 KB staging array (pinned on .NET 5+), outside the BufferManager budget. The previous transport held an 8-byte header array.
  • ManagedSession service calls: if the inner call completes while the release continuation is being registered, the reader lock is released on the thread pool shortly after, instead of before the caller resumes.
  • Concurrent ReceiveChunkAsync calls on one TcpByteTransport now throw InvalidOperationException. The channel has a single receive loop, and interleaved receives on a stream were never valid.

Not changed: discarded ResponseHeader

Every generated structure's Initialize() constructs its structure-typed fields: new ResponseHeader() (112 B), new RequestHeader() (104 B). The decoders always create the instance (new T() or the activator) and then assign every field in Decode, so the default is thrown away. That is 4.6% of the client's 2.4 KB per Read and about 3% on the server, which discards a RequestHeader per decoded request and a ResponseHeader per response it constructs. PublishResponse also discards a default NotificationMessage. Options considered:

  • Lazy getter (get => m_x ??= new X()): IsEqual/GetHashCode compare fields, so a never-read default (null) would no longer equal an explicit default. Two threads reading an unset header could get different instances. Pooled instances reset through Initialize() anyway.
  • Decode-only constructor called by the activator: only the plain Class/DerivedClass templates assign every field in Decode, not optional-field or union types. A derived type's constructor must chain to a base built by an older generator in another assembly, which lacks it (the same problem ForeignDerivedPooledExtensionClass works around). It also needs a new public marker type in Opc.Ua.Types.
  • Decode into the existing instance: needs a new IDecoder member in all decoders. It also changes behaviour when a caller decodes into an object whose header it still references.

Each touches every generated type for about 100 B per message, so this is left as is.

Harness

  • --client shared: classic sessions whose secure channels come from one ClientChannelManager.
    • Created sequentially (--connect-parallel, default 1). Concurrent Session.CreateAsync calls on one manager fail with BadSecureChannelClosed, a separate bug.
    • 20 sessions on one channel reach about 45–70k req/s, against about 140k on 20 channels.
  • ab.sh: CANDIDATE= selects the candidate build.

Testing

  • New: TcpByteTransportTests: chunks sent together, a chunk split across sends (inside the next header), a chunk larger than the staging buffer, cancellation keeping staged bytes. UaSCBinaryChannelWriteQueueTests: coalesced in order (1 + 16 + 16 + 7 sends), framing transport not coalesced, a failed batch reported for every write and later writes still sent. AsyncReaderWriterLockTests: TryEnterReader against a writer, the reader held until the operation completes and released on failure and on a completed operation, the awaiter resuming after the release.
  • Before the rebase onto master, net10.0, run sequentially: Core Stack+Security 4689 passed, 1 failed (ClientBaseTests.ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly, a meter-listener flake fixed on master by Tests: scope MeterListeners to the meter under test (fix flaky ClientBaseTests metrics test) #4708; passes 3/3 in isolation); Client 2768; Sessions 851 (379 skipped); Server 8398; Subscriptions 643.
  • After the rebase: Core Stack+Security 4811 passed; Client 2781; Stress Contract+Integration 45 passed; all 0 failed. Sessions, Server and Subscriptions were not rerun on the new base.
  • net48 build of UA.slnx: clean (0 warnings, 0 errors); dotnet format whitespace clean.

🤖 Generated with Claude Code

romanett and others added 4 commits October 11, 2026 19:53
ReceiveChunkAsync read the 8-byte chunk header and then the body, two
receive calls per chunk. On loopback, receive syscalls are a large share
of the client CPU per small request.

The transport now waits for data with a small staging buffer (8 KB,
allocated on the first receive, not from the BufferManager budget). One
receive returns the header and, when it is small and already there, the
whole chunk; bytes of following chunks are kept for the next call. A
chunk larger than the staging buffer is completed with a receive into the
rented buffer, as before. The receive buffer is still rented only after
the header has been validated, so an idle peer holds no budget; message
type and size checks are unchanged. A cancelled receive keeps the bytes
that already arrived.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…achine

Every service call was an async wrapper around the inner session call:
await the reader lock, then await the call, one more async state machine
box per request on top of the inner session's.

When services are available and no writer holds or waits for the lock,
the reader lock is now taken synchronously (AsyncReaderWriterLock
TryEnterReader) and released by a cached continuation on the inner call's
task, registered before the caller's await, so the lock is free again
when the caller resumes (unless the call completes while the continuation
is being registered; the release then runs on the pool). Otherwise the
call waits for the lock as before, so reconnect and failover still block
new calls and wait for the calls in flight. A synchronous exception from
the inner call is still reported through the returned task.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The channel writer sent queued writes one by one, one socket send each.
With several requests in flight on a channel (a shared client channel,
or a server answering concurrent requests) writes queue up behind the
one being sent.

When more writes for the same transport are queued, the writer now takes
up to 16 of them and sends their segments with one vectored send, if the
transport carries a byte stream (TcpByteTransport, through the admission
lease on the server, opts in with the internal IUaSCChunkCoalescing).
Each write is then reported on its own, in order, with its size; a
failed send is reported for every write in it, and their buffers are
returned. Writes still never start on the caller's stack, and a single
writer still sends one batch at a time, so chunk order is unchanged.
Transports that frame each send (WebSocket, capture decorators) are not
coalesced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 18:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

TCP receive cancellation can lose stream bytes, and net48 cannot cancel the new staged receive as tested.

4 open findings
What changed in this PR

Optimizes OPC UA TCP transport, ManagedSession locking, and batched writes while extending performance tooling and regression tests.

Changes:

  • Adds staged TCP receives and vectored write coalescing.
  • Adds a synchronous ManagedSession reader-lock fast path.
  • Extends transport tests and shared-channel benchmarks.
File Description
tests/​Opc.Ua.Core.Tests/​Stack/​Transport/​UaSCBinaryChannelWriteQueueTests.cs Tests coalesced writes and failures.
tests/​Opc.Ua.Core.Tests/​Stack/​Transport/​TcpByteTransportTests.cs Tests staged TCP receives.
tests/​Opc.Ua.Client.Tests/​Utils/​AsyncReaderWriterLockTests.cs Tests reader-lock fast path.
src/​Opc.Ua.Core/​Stack/​Tcp/​UaScConnectionAdmission.cs Forwards coalescing capability.
src/​Opc.Ua.Core/​Stack/​Tcp/​UaSCBinaryChannel.cs Coalesces queued writes.
src/​Opc.Ua.Core/​Stack/​Tcp/​TcpByteTransport.cs Adds staged receiving and coalescing support.
src/​Opc.Ua.Core/​Stack/​Tcp/​IUaSCByteTransport.cs Defines coalescing capability.
src/​Opc.Ua.Client/​Utils/​AsyncReaderWriterLock.cs Adds nonblocking reader acquisition.
src/​Opc.Ua.Client/​Session/​Reconnect/​ConnectionStateMachine.cs Exposes service availability.
src/​Opc.Ua.Client/​Session/​ManagedSession.Services.cs Uses the lock fast path for services.
perf/​ServerLoadHarness/​Program.cs Adds shared-channel sessions.
perf/​README.md Documents shared-client mode.
perf/​ab.sh Adds candidate-build selection.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Opc.Ua.Core/Stack/Tcp/TcpByteTransport.cs Outdated
Comment thread src/Opc.Ua.Core/Stack/Tcp/TcpByteTransport.cs Outdated
Comment thread tests/Opc.Ua.Core.Tests/Stack/Transport/TcpByteTransportTests.cs Outdated
Comment thread perf/ServerLoadHarness/Program.cs Outdated
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Code coverage

✅ Coverage gate passed.

Check Result Threshold
✅ Project line rate 88.63% (306890/346275 lines) >= 70.00%
✅ Project branch rate 79.08% >= 60.00%
✅ Patch coverage 98.58% (348/353 changed lines) >= 75.00% (> 100 changed lines)
ℹ️ Baseline delta (advisory) +15.03 pp 73.60% recorded
Uncovered changed lines
  • src/Opc.Ua.Client/Session/ManagedSession.Services.cs: 642, 643
  • src/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.cs: 1755, 1758, 1770

Coverage is above the recorded baseline - consider ratcheting coverage-thresholds.json.

Thresholds live in coverage-thresholds.json. Whole report before exclusions: line 87.76%, branch 78.28%.

@codecov

codecov Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.01700% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.86%. Comparing base (373aced) to head (fd00ba3).

Files with missing lines Patch % Lines
src/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.cs 92.00% 3 Missing and 1 partial ⚠️
...c/Opc.Ua.Client/Session/ManagedSession.Services.cs 99.12% 2 Missing ⚠️
...c/Opc.Ua.Core/Stack/Tcp/UaScConnectionAdmission.cs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4737      +/-   ##
==========================================
+ Coverage   82.84%   82.86%   +0.01%     
==========================================
  Files        2268     2268              
  Lines      346153   346275     +122     
  Branches    60358    60381      +23     
==========================================
+ Hits       286783   286949     +166     
+ Misses      39271    39218      -53     
- Partials    20099    20108       +9     
Flag Coverage Δ
actions 82.86% <98.01%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...Client/Session/Reconnect/ConnectionStateMachine.cs 86.84% <100.00%> (+0.44%) ⬆️
src/Opc.Ua.Client/Utils/AsyncReaderWriterLock.cs 95.45% <100.00%> (+1.16%) ⬆️
src/Opc.Ua.Core/Stack/Tcp/TcpByteTransport.cs 86.66% <100.00%> (+3.40%) ⬆️
...c/Opc.Ua.Core/Stack/Tcp/UaScConnectionAdmission.cs 86.30% <0.00%> (-0.40%) ⬇️
...c/Opc.Ua.Client/Session/ManagedSession.Services.cs 99.21% <99.12%> (+11.76%) ⬆️
src/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.cs 85.45% <92.00%> (+1.25%) ⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

romanett and others added 2 commits October 11, 2026 21:15
Review follow-ups:
- A receive that fails while reading the body of a chunk larger than the
  staging buffer has consumed part of the chunk, which goes back with the
  rented buffer. The transport now remembers that and fails later
  receives with BadTcpInternalError instead of reading body bytes as the
  next chunk header. Cancellation while the bytes are in the staging
  buffer still resumes.
- On .NET Framework the staging receive takes no token; the bytes it
  returned are now kept before the cancellation is observed. The
  partial-header cancellation tests run where ReceiveAsync is
  cancellable (.NET 5+).
- Tests send their whole payload (SendAsync may send less), and the
  harness wraps the connect-parallel line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the receive guard

Patch coverage follow-up: CreateSession/ActivateSession/CloseSession
passthrough cases; a service call waiting while a writer holds the
service lock; a call that fails before the inner call starts faulting the
returned task and releasing the reader lock; an unexpected exception in a
coalesced send reported as BadTcpInternalError for every write in it; a
second concurrent ReceiveChunkAsync rejected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@marcschier marcschier added the ready Ready to merge once CI Passes label Oct 11, 2026
@romanett
romanett merged commit 2c431ad into master Oct 12, 2026
118 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready Ready to merge once CI Passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants