Commit Graph

2 Commits

Author SHA1 Message Date
romanett b25429bccc Client: cut per-request allocation and CPU in the channel and session; measure the client in the load harness (#4720)
Follows #4707 (replaces #4714, which closed when the #4707 branch was
deleted). Profiling the client process under load showed per-request
allocation spread over the channel write path, the response wait,
per-chunk header encoders and an AES instance per message, plus a stack
walk per service call. This PR removes them and extends the load harness
to measure the client.

## Results

Paired A/B against the #4707 head, the server work now on master (3
alternating rounds, medians, 20 sessions unless noted, workstation GC on
both sides, 24-core machine). Client = the harness process, server = the
reference server child.

| Scenario | Throughput | p50 | p90 | Client CPU / op | Client alloc /
op | Server alloc / op |
|---|---|---|---|---|---|---|
| Read, 1 value | **+21%** (141k → 170k req/s) | −9% | −11% | −13% |
−46% (4.5 → 2.4 KB) | −20% |
| Read, 1 value, ManagedSession | **+18%** | −9% | −12% | −18% | −44% |
−20% |
| Read, 1 value, SignAndEncrypt | **+28%** | −14% | −20% | −24% | −52% |
−26% |
| Read, 1 value, 200 sessions | **+29%** | −11% | −17% | −17% | −46% |
−20% |
| Read, 100 values | +5% | −2% | −3% | −6% | −44% (15.6 → 8.8 KB) | −2%
|
| Read, 100 values, SignAndEncrypt | +8% | −7% | −2% | −2% | −46% | −3%
|
| Write, 100 values | ±0 | ≈ | ≈ | −14% | **−63%** (10.8 → 4.0 KB) | −2%
|
| Browse (Server object) | **+12%** | −7% | −11% | −19% | −17% | −12% |
| Data changes, classic fast callback | ±0 ¹ | ≈ | ≈ | −21% | −10% | −5%
|
| Data changes, classic item events | ±0 ¹ | ≈ | ≈ | ≈ ² | −7% | −5% |
| Data changes, ManagedSession (V2) | ±0 ¹ | ≈ | ≈ | ±0 | −5% | −5% |
| Data changes, SignAndEncrypt | ±0 ¹ | ≈ | ≈ | −22% | −14% | −7% |

¹ Throughput is bounded by the writer rate; latency is dominated by the
100 ms publishing interval. Per notification the client uses about 2 µs
and 170–260 B.
² The client uses 0.2–0.4 cores in the subscription scenarios; the CPU
per notification is within run-to-run noise there.

Client GC pause time in the window drops 38–66% in the Read and Write
scenarios (17% for Browse). About 70% of the remaining client CPU per
small request is kernel time in loopback socket send/receive.

## Changes

- **Activity source**: `Session`, `SessionClientBatched`, the classic
`Subscription` and `ClassicSubscriptionEngine`, and the TCP/HTTPS client
channels resolve their `ActivitySource` once.
`TelemetryExtensions.StartActivity` called
`Assembly.GetCallingAssembly()` (a stack walk) on every service call,
~8% of client CPU.
- **Channel write queue** (`UaSCUaBinaryChannel`, client and server): an
ordered queue drained by one thread-pool writer replaces the per-write
`ContinueWith`/`Unwrap`/trailing continuation chain. The writer is
queued to the issuing thread's local pool queue, like the task
continuation was (queuing it globally cost ~15 µs of p50 in a first
iteration). Successful writes are reported inline instead of through
`Task.Run`; failed writes still go through the pool. The client channel
enters its gate only for a failed write instead of on every write
completion.
- **`TcpByteTransport`**: no linked `CancellationTokenSource` when the
caller's token cannot be cancelled (the channel always passes `None`); a
single-chunk `BufferCollection` uses the `ReadOnlyMemory` send instead
of the vectored `Task<int>` send; `PoolingAsyncValueTaskMethodBuilder`
on .NET 6+ for the receive/send state machines; no closure per vectored
send.
- **`BinaryEncoder.GetNodeIdEncoding`**: `Convert.ToUInt32(uint,
IFormatProvider)` bound to the `object` overload and
`Convert.ToByte(enum, ...)` boxed the enum, two boxes per encoded
NodeId. 44% of the client allocation of a 100-value Read; the server
encodes NodeIds the same way.
- **Symmetric chunk headers**: the 24 header bytes are written and read
with `BinaryPrimitives` instead of a `MemoryStream` +
`BinaryEncoder`/`BinaryWriter` (send) and `BinaryDecoder`/`BinaryReader`
(receive) per chunk. A chunk too short for a field still gets the
decoder's `BadDecodingError`.
- **AES-CBC**: `ChannelToken` keeps one `Aes` per direction, used with
the one-shot `EncryptCbc`/`DecryptCbc` on .NET 6+ (net48 reuses the
instance and creates the transform). Previously every message created an
`Aes`, imported the key (`BCryptImportKey`) and created a transform,
~15% of client CPU with SignAndEncrypt. The instance is taken out of the
token while in use, so concurrent callers never share one. The public
`CryptoUtils` overloads are unchanged.
- **`ChannelAsyncOperation`**: `SendRequestAsync` waits on an internal
task-free completion source instead of `EndAsync`
(`TaskCompletionSource` + `Task` + `WaitAsync` promise + state machine);
`EndAsync` registers cancellation instead of `WaitAsync(ct)`; the
timeout timer uses a static callback; completing with `default(T)`
shares a box.
- **Session keep alive**: every good response called `Timer.Change`
under the session lock, taking the timer queue lock. It now records the
time; the timer callback defers itself to one interval after the last
response, which is when the per-response reset made it fire.

### Behaviour notes
- `TcpByteTransport.SendChunkAsync`/`ReceiveChunkAsync` return pooled
`ValueTask`s on .NET 6+: like any `ValueTask` they must be awaited once.
- A successful write's `HandleWriteComplete` now runs on the writer
instead of a separate pool work item. Overrides outside the stack that
block in it delay later writes on that channel.
- The write itself no longer flows the caller's `ExecutionContext`
(`Activity.Current` is not set inside the transport send).

## Harness (`perf/`)
- `client-process:` and `client-per-op:` result lines; `--trace-side
client` records the harness itself.
- `--client managed` (ManagedSession, V2 engine) and `--callback item`
(classic per-item events).
- Classic fixture sessions no longer request diagnostics in every
response (the server otherwise sends a `DiagnosticInfo` per
notification).
- `ab.sh`: `SCENARIOS`/`ROUNDS`/`OUT`, CRLF tolerant, defaults before
the scenario (a `--sessions 200` scenario silently ran with 20 before);
`client-scenarios.txt`; `ab-summary.ps1` reports client and server
medians.
- README notes two profile traps: `Monitor.Enter_Slowpath` in the
sampled CPU profile without matching contention events (46 in 10 s
here), and allocation ticks over-attributing small objects allocated
after large arrays.

## Testing
- New: `UaSCBinaryChannelWriteQueueTests` (order, one send at a time,
never on the caller's stack, failures reported and later writes still
sent, concurrent writers), `ChannelAsyncOperationWaitTests` (late/early
completion, faults, cancellation, disposal, continuation off the
completing thread, 2000-iteration completion/cancellation race,
timeout), `CryptoUtilsAesCacheTests` (cached = fresh cipher text across
IVs, round trips, concurrent users, token disposal),
`TcpByteTransportTests` single/multi-buffer and cancellable sends,
`SessionKeepAliveTests` keep-alive read exactly one interval after the
last response.
- After merging master: Core Stack 3571 passed, Client 2763 passed, 0
failed.
- Before merging master, net10.0: Core Stack+Security 4680 passed;
Client 2763; Sessions 849 (379 skipped); Server 8391; Subscriptions 643;
all 0 failed.
- Stress Contract+Integration: 5 failures, the same 5 on the unchanged
#4707 head (`CertRotationContractTests`, `GateAndBypassTests`,
`LeaseLifecycleTests`, `RetryBudgetEnforcementTests` reflection seam,
`CertRotationLiveTests`), not caused by this PR.
- net48 build of `UA.slnx` clean; `dotnet format whitespace` clean.

## Not changed (follow-ups)
- Two `recv` calls per chunk (8-byte header, then body). Merging them
needs read-ahead with carry-over, which the transport avoids so that
idle peers do not hold receive buffers.
- `ManagedSession` service wrappers add an async state machine and a
reader lock per call (~200 B).
- Generated responses allocate a discarded `ResponseHeader`
(generator-wide, as noted in #4707).
- Decoded `MonitoredItemNotification` objects and the V2
`DataValueChange[]` per message dominate notification allocation
(`WithPoolNotifications` pools the latter).
- Coalescing queued writes on one channel into one vectored send would
cut syscalls under concurrent requests on a shared channel.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Marc <marcschier@hotmail.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
2026-10-10 20:00:33 +02:00
romanett 28f1401324 Server: cut request-path lock contention and allocation; add load harness (#4707)
Profiling the reference server under load showed most request-path CPU
spent spinning on contended locks, plus several avoidable allocations
per request or per node. This PR removes them, switches the reference
server to Server GC, and adds the load harness used to measure it.

## Results

Paired A/B against `master` (3 alternating rounds, medians, 20 sessions,
workstation GC on both sides, 24-core machine):

| Scenario | Throughput | p50 | p90 | Server CPU / op | Server alloc /
op | Lock contention |
|---|---|---|---|---|---|---|
| Read, 1 value | **+45%** (94k → 137k req/s) | −20% | −46% | −37% |
−39% | −94% |
| Read, 100 values | **+40%** | −26% | −17% | −24% | −25% | +130% ¹ |
| Read, 100 values, SignAndEncrypt | **+48%** | −36% | −20% | −30% |
−26% | +106% ¹ |
| Browse (Server object) | **+33%** | −31% | −12% | −13% | −14% | −40% |
| Write, 100 values | +13% | −12% | −4% | −10% | −19% | −23% |
| Data-change subscriptions | ±0 ² | ≈ | ≈ | −14% | −17% | −29% |

¹ The faster server spends more time in the remaining per-node locks.
² Throughput is bounded by the writer rate in this scenario.

Separately, Server GC instead of workstation GC doubles Read-100
throughput (21k → 44k req/s) and cuts p90 from 2.1 ms to 0.6 ms, because
workstation GC paused the server for about half the wall time under
load. The working set grows from ~210 MB to ~430 MB.

## Changes

- **`ObjectPool<T>`**: lock-free slot array. `ConcurrentBag.Count` took
every per-thread lock on each `Return` of the pooled response value-task
source, once per request.
- **`FairRequestQueue`**:
- drops redundant gate entries: the fail-fast pre-checks and the stopped
check read volatile fields and are re-checked under the gate; the owner
count is folded into selection;
- coalesces wake pulses with a pending flag, so the bounded signal
channel's lock is only taken when no token is outstanding. Wake-count
semantics in the tests are unchanged.
- **`DefaultServerResourceIsolationProvider`**:
  - compares peer addresses without `GetAddressBytes()` allocations;
- builds the unsecured verification context only when a classifier is
configured.
- **TCP listener / channel**: caches the certificate DER copies per
channel, keyed on the certificate instance. `X509Certificate2.RawData`
returns a fresh copy on every access, and the listener read it for every
request.
- **`EndpointBase`**: resolves the request `ActivitySource` once instead
of `Assembly.GetCallingAssembly()` per request.
- **Node managers**:
- one `ServerSystemContext` copy per request (cached on the
`OperationContext`) for per-node callbacks: permission metadata, node
metadata, view membership, method lookup;
  - removes a discarded copy in `GetReferenceDescriptionAsync`;
- per-request operation caches are plain `Dictionary` instead of a
`ConcurrentDictionary` with 24 lock objects;
  - the validation cache is built in one pass;
  - fixes zero-capacity error lists.
- **`ResourceManager`**: translation lookups read an immutable snapshot
that is rebuilt after writes. Browse serialised on the global lock for
every display name.
- **`MonitoredItem`**: shares the default `DataChangeFilter` instead of
allocating one per sampled value.
- **Reference server**: `ServerGarbageCollection=true`; new "Garbage
collector mode" section in `docs/ServerScalability.md`.
- **`perf/`**: `ServerLoadHarness` (out-of-process server plus load
driver, server-side CPU/GC/alloc/contention deltas, optional
`dotnet-trace` capture), `TraceAnalyzer` (TraceEvent-based
CPU/allocation/contention aggregation), and paired A/B scripts. Not part
of `UA.slnx`.

### Behaviour notes for custom node managers
- The `IDictionary<NodeId, NodeState>` operation cache handed to
overrides is now a `Dictionary`. It was a `ConcurrentDictionary`, but
only one request flow uses it.
- Within one request, per-node callbacks of a node manager share one
`ServerSystemContext` copy.

### Later commits on this PR
- **Permission checks without `NodeMetadata`** (`9068684c3`): the
role-permission and access-restriction validators run on an internal
`PermissionMetadata` struct; `AsyncCustomNodeManager` fills it on the
stack from the same attributes and namespace defaults. Browse's
reference filter reads 3 attributes per target instead of 13. Read 100
values: 120.6 → 95.7 KB/request, ~21k → 26k req/s; Browse 67.6 → 51.8
KB, 26.7k → 29.8k req/s.
- **Read-only channel evidence** (`936b28d76`, `dc638ab9f`):
`SecureChannelContext.ClientChannelCertificate`,
`ServerChannelCertificate` and `ChannelThumbprint` are `ByteString`, so
the cached per-channel certificate buffer is shared without a copy and
cannot be altered by handlers or classifiers. **Public API change**, see
the new section in `docs/MigrationGuide.md`. The isolation provider's
revalidation snapshot still copies.
- **Session lookup** (`936b28d76`):
`SessionManager.TryGetSessionContext` takes the candidate snapshot
without the bindings lock; the validating check under the lock is
unchanged (2 instead of 4 global lock acquisitions per request).
- Unit tests for `ObjectPool`, the per-request system context cache, the
translation snapshot, `SecureChannelContext` null semantics and a
stopped `FairRequestQueue`; `perf/` follows the repository conventions.

## Testing
- `Opc.Ua.Server.Tests` (net10.0): 8386 passed before the `ByteString`
change. After it, 13 secure-session and isolation tests failed; both
causes are fixed in `dc638ab9f` and those tests (25 with their variants)
pass. A full rerun of the Server, Sessions, Core, Redundancy, LDS and
GDS suites on the final commit is in progress.
- Full solution builds for net10.0 and net48; `dotnet format whitespace`
clean.

### o6-automation opcua-benchmarks (sampling, deep pipelines)
Replicated both tests of the [o6-automation
benchmark](https://github.com/o6-automation/opcua-benchmarks) in the
harness (`--scenario counters`, and `read` with 10 clients × N
outstanding), paired against `origin/master`:

| Test | master | this PR |
|---|---|---|
| Subscription sampling, 100 Hz, every item ≥ 99 % of samples, 1 item |
FAIL (75–94 %) | PASS (100 %) |
| same, 1024 items | FAIL (64–73 %) | PASS (99.9 %) |
| same, capacity | — | between 4096 and 6144 items (~410–520k samples/s)
|
| Read, 10 clients × 100 outstanding, None | stalls (0–1.7k req/s) |
121–131k req/s |
| Read, 10 clients × 100 outstanding, SignAndEncrypt | stalls (0 req/s)
| 89–91k req/s |
| Read, 10 clients × 1000 outstanding | channels faulted
(`BadTcpNotEnoughResources`) | 0 errors, ~45–54k req/s |

- **Fixed-rate sampling** (`30b769d13`): the sampling loop waited one
interval after each sample, adding timer overshoot and sampling time to
every cycle; samples now have absolute deadlines.
- **Async response send** (`cdac5c5ca`): `SendResponse` blocked
thread-pool workers on the channel gate; with many requests in flight
the receive loop that was handed the gate waited for a pool thread and
the channel stalled. The TCP, Kestrel TCP and WebSocket listeners now
use `SendResponseAsync`.
- **Backpressure instead of channel fault** (`191cd2ba7`): server
channels stop reading while more than 100 responses are queued and
resume at 50, instead of faulting the channel. Client channels keep
reading. At 10,000 outstanding requests throughput is ~50k req/s with or
without the limit engaged, so the remaining gap to the 100-per-client
case is not the backpressure.

## Not changed
- `SentMessageQueue` deep-clones every `NotificationMessage`. Retained
messages return their pooled payloads on acknowledgement while the
response copy may still be queued for send, and a client can acknowledge
a sequence number before receiving it; the clone keeps a recycled
payload from reaching another session's response. Removing it needs a
send-completion hook from the transport.
- Generated responses allocate a default `ResponseHeader` that is
replaced (~2-3 % of per-request allocation). Fixing it means making
structure fields of every generated type lazily initialized, a
generator-wide change to default values, equality and encoding.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-09 13:00:49 +02:00