6 Commits

Include renames

Author	SHA1	Message	Date
romanettandClaude Opus 5.5	40288fa7d8	Tests: weighted FairRequestQueue test asserts the median of three windows (#4763) ... # Description `FairRequestQueueTests.WeightedServiceHoldsUnderConcurrentWorkersAsync` failed once on GitHub Actions macOS net10.0 (#4753, run 38349863236): light=124 heavy=276, ratio 2.23 against an expected 2.5-3.5. It passed on every other CI run and 30/30 locally. **Root cause.** `TrySelect` dequeues an owner's turn token from the lock-free ready queue. On the last token of a turn, `TakeTurnToken` CASes the turn and calls `PushTurn` for the next one. A worker preempted between those two steps leaves the owner with no queued token while it is stalled. The work-conserving dispatch serves the other owner in the meantime, and that share is never made up. Delay injection confirms this: sleeping 1 ms on every 7th turn publish collapses 1:3 to roughly 1:1 (light=199 heavy=201). Publishing a turn ahead of time would only cover stalls shorter than one round (microseconds), while real preemption lasts milliseconds. Making up the lost share (deficit or virtual time) would mean redesigning the scheduler. So this PR documents the bounded skew and makes the test robust instead. **Changes** - `FairRequestQueue` remarks: the weighting holds over time, not in every window, and a preempted worker can cost an owner its share for the length of the stall. - `WeightedServiceHoldsUnderConcurrentWorkersAsync` now measures 3 independent windows through `MeasureWeightedServiceAsync` and asserts that the **median** heavy/light ratio is in 2.5-3.5. The failure message lists all three windows. - Each window uses perOwner = 480 and measured = 560, with queue limits of `2 * perOwner`. That stays under the TestProvider's 1000-request cap and keeps both owners backlogged. - Each window still checks that both owners stay backlogged, then drains the queue and checks that everything is back to zero. The median tolerates one skewed window and still fails when the weighting is lost systematically. With stalls injected into one window only, the median is 3.0 and the test passes (e.g. `light=238 heavy=322; light=140 heavy=420; light=140 heavy=420`). With stalls injected into every window, it still fails. **Validation:** the `FairRequestQueueTests` fixture passes 93/93 on net10.0 Release. The weighted test passed 30/30 runs pinned to 2 cores (`start /affinity 3`). `dotnet format whitespace --verify-no-changes` is clean, and the build has 0 warnings. ## Related Issues - Flaky test seen on #4753 after #4750 (FairRequestQueue without a queue-wide lock) ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-13 21:48:18 +02:00
romanettandClaude Opus 5.5	a14b23e7dd	Server: cheaper symmetric crypto path, FairRequestQueue without a queue-wide lock (#4750) ... # Server: cheaper symmetric crypto path, FairRequestQueue without a queue-wide lock Roadmap items E1 and 5b of the o6 benchmark gap analysis, one commit each. - **E1:** Windows one-shot `Aes.EncryptCbc`/`DecryptCbc` import the key (`BCryptImportKey`) on every message, although #4720 caches one `Aes` per channel token. The AES-GCM and ChaCha20-Poly1305 policies also created a new keyed instance per message. - **5b:** with `AsyncCustomNodeManager`, which is what the o6 benchmark's 2.0.1 worker uses, the `FairRequestQueue` gate became the largest waiting point. `TryEnqueue`, `TrySelect` and `TryDequeue` spun on one `Lock` for every request. No wire, configuration or public API changes. ## Results These are paired A/B runs against `origin/master`, five rounds per scenario. The two harness builds alternate per round, starting side swapped every round, on the same shared machine (Ryzen AI 9 HX 370, 12 cores / 24 threads, Windows 11). The tables show medians, with ranges in brackets where they matter. The server is configured like the o6 2.0.1 worker: `AsyncCustomNodeManager`, Server GC, queue sized clients × outstanding + 3. o6read uses the benchmark's native open62541 client workers; `read` uses the harness's .NET client. On this machine throughput moves between rounds by up to 2×, so CPU and bytes per request are the reliable signals. ### E1: symmetric crypto (measured on 0dff94bd3) | Scenario | req/s | Server CPU µs/req | Server alloc B/req | Client CPU µs/req | Client alloc B/req | |---|---|---|---|---|---| | read --nodes 1, Basic256Sha256 SignAndEncrypt, 24 cores | 115,300 → 103,600 (ranges overlap) | 64.6 → 60.9 (**−6 %**) | 5,698 → 5,486 | 44.9 → 41.7 (**−7 %**) | 2,672 → 2,456 | | o6read 10×100, encrypted, 4-CPU mask | 71,300 → 75,900 | 51.8 → 51.4 | 6,323 → 6,111 | – | – | | o6read 10×100, encrypted, 24 cores | 123,500 → 118,200 (ranges overlap) | 115.7 → 117.2 | 6,716 → 6,515 | – | – | | o6read 10×100, unencrypted, 4-CPU mask (control) | 79,100 → 70,600 | 48.0 → 48.4 | 6,088 → 6,090 | – | – | Isolated costs per call (one pinned core, 224-byte chunk, variants alternating in one process): | Operation | Before | After | |---|---|---| | AES-256-CBC encrypt | 778 ns, 128 B (one-shot) | 329 ns, 24 B (cached transform) | | AES-256-CBC decrypt | 682 ns, 128 B | 239 ns, 24 B | | AES-256-GCM seal | 1,022 ns, 72 B (new `AesGcm`) | 260 ns, 0 B (cached) | | HMAC-SHA256 | 306 ns (per-token instance, unchanged) | – | For comparison, `HMACSHA256.TryHashData` costs 458 ns and `IncrementalHash` 318 ns. **E1 does not close the 20–50 µs encrypted gap, and no crypto change could.** Pinned to four CPUs, encryption costs master only about 3–4 µs per request (51.8 vs 48.0 µs). The extra cost at 24 cores comes from threads spinning on locks while signing and encryption run inside the channel gate (roadmap A1). The ~7 µs of HMAC in the report's profile is a thread-time artefact. `HashProviderCng.AppendHashData` shows 40× more samples on the sign side, inside the gate, than on the verify side for the same chunk sizes. Threads are preempted there while spinners hold the CPUs. ### 5b: FairRequestQueue (unencrypted, E1 has no effect here) | Scenario | req/s | Server CPU µs/req | Server alloc B/req | Contention events/s | |---|---|---|---|---| | o6read 10×100, 24 cores | 149,000 → 150,200 | 110.8 [106–117] → 99.6 [96–109] (**−10 %**) | 6,404 → 6,367 | 4,533 → 2,125 | | o6read 10×1000, 24 cores | 152,200 → 161,200 | 106.7 → 96.3 (**−10 %**) | 6,421 → 6,358 | 4,131 → 1,921 | | read --nodes 1 --sessions 200, 24 cores | 126,100 → 141,200 (+12 %) | 53.6 → 51.1 (−5 %) | 5,353 → 5,294 | 4,078 → 455 (**−89 %**) | | o6read 10×100, 4-CPU mask | 85,600 → 83,900 | 45.7 [44.1–47.8] → 46.8 [44.4–48.9] (noise) | 6,089 → 6,060 | 281 → 176 | Traces of o6read 10×100 at 24 cores, one run per side: - **Lock spinning.** In the sampled CPU profile, frames above `Lock.TryEnterSlow` in the fair queue fell from **16.6 % of on-CPU samples** (`TryEnqueue` 5.8, `TrySelect` 5.8, `TryDequeue` 4.9) **to none**. - **Contention events in the window:** 15,331 → 5,879. - **What remains of the queue:** - the wake channel in `DequeueAsync`: idle workers parked on the coalesced signal, 1.3 % of on-CPU samples, kept unchanged; - `SessionManager.TryGetSessionContext`'s server-wide `m_bindingsLock`, which every request takes at admission and again at dispatch. It is now the largest spin point (~26 % of `Lock.TryEnterSlow` samples) and is a separate follow-up. The channel gate (A1) is another ~17 %. In the o6 configuration all ten sessions map to **one** owner, so per-owner locks would not have helped. An intermediate version with a lock per owner FIFO left 12.8 % of the samples on those locks. That is why the FIFO itself is lock-free. ## Changes ### Symmetric cipher cache (E1) - `SymmetricCipherCache` replaces `AesCache`; a `ChannelToken` keeps one per direction (`ServerCiphers`, `ClientCiphers`). Per slot it retains: - a CBC encryptor keyed with the token's key and IV; - a CBC decryptor keyed the same way; - the AES-GCM or ChaCha20-Poly1305 instance. - **How a CBC message is processed:** the cached `ICryptoTransform` transforms the whole message in place with `TransformBlock`. An empty `TransformFinalBlock` (padding `None`) then rewinds the chaining state to the IV without re-keying, on CNG, OpenSSL and .NET Framework CAPI alike. Cached and fresh ciphertext are tested equal. - **Concurrency and ownership:** - A cipher is taken out of its slot while in use, so concurrent senders never share one. - It is matched to the key and IV by reference, and replaced if the key material differs. - It is disposed instead of returned after a failed transform, or once the token has been disposed. - **Unchanged:** - The uncached public `CryptoUtils` overloads keep the one-shot path. - The order is still verify, then decrypt, then padding, so a tampered padding byte under a valid signature is still reported as padding, after the signature check. - HMAC is unchanged. ### FairRequestQueue (5b) - **No lock in admission or dispatch.** - Each owner has a lock-free FIFO: Michael–Scott, with the entries themselves as the nodes, so queueing allocates nothing. The `LinkedListNode` per request is gone. - A request is claimed exactly once, by a compare-and-swap from selection, cancellation or stop. Claimed entries are dropped lazily when they reach the head. - **Weighted round robin over a lock-free ready queue of turn tokens.** - A turn holds one token per queued request, up to the owner's weight. A weight of 1,000,000 therefore costs no more scheduling state than the backlog. - Taking a turn's last token queues the owner's next turn at once, behind the other owners. The turn state is one 64-bit word changed by CAS, so the service order is the token order even while workers race. Weights 1:3 under 8 concurrent workers stay within 2.5–3.5. - **Failed execution admission:** the head goes to a small sorted list under the owner's gate, served before the FIFO. Its owner is not retried before capacity changes, as before. - **Owner FIFO retirement:** an owner FIFO is retired by an atomic CAS of its count from 0 to a sentinel, and a retired FIFO can never be counted again. The next request of that owner starts a new FIFO, and the retired FIFO's leftover tokens are skipped. - **Unchanged:** - The count bound is reserved with `Interlocked` before linking. - Global FIFO mode still selects one at a time, so a blocked head cannot be overtaken. - Wake coalescing (#4707), parked-request decoupling and `ServerBase.RequestQueue`'s worker growth. ## Behaviour notes - **Cipher cache memory:** a token now keeps up to two CBC transforms (plus their `Aes`) or one AEAD instance per direction, instead of one `Aes`. They are released with the token. - **Weighted ordering under concurrency:** dispatch order across owners follows token order. Within an owner, admission order is kept, including requests that fail execution admission and go back to the front. - **Idle scheduling state:** cancelled requests leave stale turn tokens in the ready queue until the next dispatch pass. They are discarded at once when the last queued request is cancelled. - **net48:** the BouncyCastle-backed AEAD types are cached like the platform ones. ## Testing - **New and extended tests:** - `CryptoUtilsAesCacheTests`, for all CBC and AEAD policies (Basic128Rsa15, Basic256, Basic256Sha256, Aes128_Sha256_RsaOaep, Aes256_Sha256_RsaPss, ECC_nistP256, ECC_nistP256/P384/RSA_DH AesGcm, ChaChaPoly): - cached vs fresh ciphertext for 0–300-byte bodies; - IV or key change; - tampered signature, ciphertext, tag and header, each rejected with the cache recovering; - bad padding under a valid signature; - 256 concurrent senders; - token disposal during a transform, and a return racing disposal. - `FairRequestQueueTests`: - turn tokens bounded by the backlog, not the weight; - cancellation leaves no tokens behind; - 1:3 weights under 8 workers; - no lost wakeups (4 producers, 8 waiting workers, both modes); - cancellation racing dequeue completes or dispatches each request exactly once and returns every lease; - stop while workers wait and admissions race; - a 50-owner, 16-worker stress run with cancellations; - pipelined clients (1×200, 10×100, 64×1) with ample and with scarce execution capacity. - The last test stalls on an earlier version of this change that briefly marked an emptying owner as retired, when the race window is widened with an injected delay. That bug stalled o6read 10×100 every time in the harness; a dump showed 1,000 queued requests with no turn tokens. Without the delay the window is too narrow to hit at unit level. - All 90 FairRequestQueue tests pass 30/30 repeated runs on net10.0 and 5/5 on net48. - **Suites,** net10.0, after rebasing on 8b360da7d (#4748): | Suite | Passed | Failed | Skipped | |---|---|---|---| | Opc.Ua.Core.Tests | 6,357 | 0 | 88 | | Opc.Ua.Server.Tests | 8,483 | 0 | 5 | | Opc.Ua.Sessions.Tests (all secure-channel policies) | 851 | 0 | 379 | | Opc.Ua.Stress.Tests Contract + Integration | 45 | 0 | 0 | | Opc.Ua.Interop.Tests (1.5.378 peer, SignAndEncrypt Basic256Sha256, Aes256_Sha256_RsaPss, ECC_nistP256) | 218 | 0 | 29 (AEAD policies the 1.5.378 peer does not offer) | - **net48:** Opc.Ua.Core.Tests builds, and CryptoUtilsAesCacheTests plus FairRequestQueueTests pass. `dotnet format whitespace` is clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-13 06:28:12 +02:00
romanettandClaude Opus 5.5	25ebf5a4d8	Server: remove the server-wide locks from the request pipeline, bounded transport buffer pool (#4742) ... # Server: remove the server-wide locks from the request pipeline, bounded transport buffer pool Quick wins #5 and #6 of the o6 benchmark gap analysis. Under 10 channels × 100 outstanding single-value Reads on 24 cores, about 59 % of the server's on-CPU time spun in `Monitor`/`Lock` slow paths, and roughly 20 of the ~30 lock acquisitions per Read were on server-wide locks in the request pipeline. Separately, on hosts with one or two processors every response allocated a fresh 64 KiB chunk, because `ArrayPool<byte>.Shared` runs out with about a thousand responses in flight. No wire or configuration changes. The only public API change is an additive `RequestLifetime` constructor. The admission *sizing* (`MaxQueuedRequestCount` → shared capacity and reserved floors) is untouched; this PR only changes how the accounting is locked. ## Results Paired A/B against `origin/master` (f5062be34): the two harness builds alternate per round on the same machine, the server runs Server GC, and the table shows medians. o6read uses the o6-automation benchmark's native open62541 client workers (10 clients × 100 in flight); the other rows use the harness's .NET client with 20 sessions. | Scenario | Rounds | req/s | Server CPU µs/req | Alloc B/req | p99 | |---|---|---|---|---|---| | o6read 10×100, 24 cores | 4 | 199,800 → 284,800 (**+43 %**) | 65.7 → 44.5 (**−32 %**) | 7,732 → 7,833 | – | | o6read 10×100, 4-core mask | 6 | 105,300 → 102,600 (−3 %, ranges overlap) | 37.8 → 38.8 (noise) | 7,824 → 7,636 | – | | o6read 10×100, 1 core (`DOTNET_PROCESSOR_COUNT=1`) | 3 | 19,750 → 41,070 (**+108 %**) | 50.1 → 24.2 (**−52 %**) | **55,345 → 7,428** | – | | read --nodes 1 --inflight 50, 24 cores | 4 | 250,200 → 355,100 (**+42 %**) | 50.3 → 31.0 (**−38 %**) | 6,551 → 6,448 | 8.8 → 10.3 ms | | read --nodes 1 --inflight 50, 1 core | 3 | 45,300 → 89,550 (**+98 %**) | 224 → 130 (**−42 %**) | 6,985 → 6,748 | 29.3 → 19.6 ms | | read --nodes 100, 24 cores | 4 | 47,940 → 48,260 (+1 %) | 336 → 324 (−4 %) | 90,520 → 90,470 | 1.42 → 1.42 ms | | browse, 24 cores | 4/3 | 53,110 → 51,840 (−2 %) | 214 → 218 (+2 %) | 44,450 → 44,280 | 1.92 → 1.58 ms | Contention (`--trace-profile contention`, o6read 10×100, 24 cores, one run per side): the isolation provider, `FairRequestQueue`, the request queue's worker gate and `RequestManager` went from **26 % of all contention events (~42k in the window) to 1.2 % (~2k)**, at 40 % higher throughput. What remains is `CustomNodeManager2`'s lock and the dispatcher's permission path (separate roadmap items). In the sampled CPU profile, `Lock.TryEnterSlow` fell from 43 % to 2.7 % of on-CPU time. The 4-core rows are within noise: with four processors these locks were not the bottleneck. The read `--inflight 50` p99 at 24 cores rose with 42 % more requests in flight per second; at 1 core it fell by a third. ## Changes ### Bounded transport buffer pool (#6) - `BoundedArrayPool` (internal): power-of-two buckets in lock-free `ConcurrentQueue`s, at most 1024 arrays and at most 64 MiB in the largest bucket (1024 × 64 KiB for the default `MaxBufferSize`), retained memory below 2 × 64 MiB. Arrays are retained only once returned, so a pool grows to the peak concurrency it has seen. - One instance per maximum array length is shared by every client and server buffer manager of the process, so client channels do not each keep their own arrays. - `ArrayPoolBufferManagerBase` uses it instead of `ArrayPool<byte>.Shared` for buffer sizes up to 1 MiB (larger sizes keep their configurable pool). `GetExpectedBufferSize` no longer doubles for the next bucket on .NET Framework, since the bounded pool allocates the exact bucket when it is empty. ### Isolation provider (#5) - No provider-wide lock on `TryAcquire`/release. Stage counters, owner usage and protected floors are reserved with bounded compare-and-swap loops that never pass a limit; counters already reserved are returned if a later one is full. - The owner table is a `ConcurrentDictionary`. An owner that already holds a lease of the same accounting class is admitted lock-free. A class count crosses zero only under a table lock, which keeps the exclusive class, the protected table slots and entry removal consistent with the leases. ### FairRequestQueue (#5) - The selected head is unlinked when it is selected, so an admitted dispatch takes the gate once instead of twice. - In weighted mode several workers select at once, instead of one selector while the others go back to wait for a wake signal. - A head that finds execution capacity exhausted is relinked in admission order, and its owner is not retried before capacity changes (as before). - Global FIFO mode keeps one selection at a time, so its blocked head still cannot be overtaken. ### ServerBase.RequestQueue (#5) - Fair mode no longer takes the worker gate in `ScheduleIncomingRequest`. - `GrowFairWorkers` pre-checks the counters and takes the gate only to add a worker. - Running handlers are counted with `Interlocked`; shutdown's drain is created after the workers exit and completed by the last handler. ### One cancellation source per request - New public constructor `RequestLifetime(TimeSpan timeout, CancellationToken first, CancellationToken second)`. It links only the tokens that can be cancelled (a duplicate token once) and applies the timeout with `CancelAfter` on the same source. `EndpointIncomingRequest.CallAsync` uses it instead of a `CancellationToken[]`, a separate TimeoutHint source and a linked source over three tokens. The lifetime drops from 528 B to 240 B, and a timeout still reports `Bad_Timeout`. - `ServerBase.RequestQueue` no longer links the transport token again for requests that link it themselves (internal `ILinksScheduledCancellation`, implemented by `EndpointIncomingRequest`). Other `IEndpointIncomingRequest` implementations keep the linked token. - Paired against the lock changes alone (4 rounds, 24 cores): allocation −4 % (o6read 7,823 → 7,539 B, read `--nodes 1 --inflight 50` 6,446 → 6,164 B); CPU and throughput unchanged within noise. ### RequestManager (#5) - Executing requests and validation scopes are `ConcurrentDictionary`s; registration and completion take no lock. - Drains publish a count before they snapshot, and a completion takes the lock only when a drain is registered. - The request timer is stopped only if no deadline was registered meanwhile. - The validation scope's `AsyncLocal` was write-only and is removed. The request scope skips the ambient id when the dispatch scope already carries it: 4 fewer `ExecutionContext` allocations per request. ## Behaviour notes - **Admission races:** two acquisitions racing for the last units of a stage may both be rejected where a serialized order would admit one. No limit is ever exceeded, and the usage gauge never reads above capacity. - **Weighted-mode ordering:** concurrent selections of one owner may dispatch in a different order when one of them fails execution admission while the other succeeds. Blocked requests are relinked in admission order. Global FIFO mode is unchanged. - **Cancellation and stop:** a request cancelled, or a queue stopped, while the request's admission runs is completed exactly once by the selecting worker (`Bad_RequestCancelledByClient` / `Bad_ServerHalted`), as before. - **Session close and Cancel:** registration ends with a full fence. A Cancel records its pending entry before it scans the registered requests, so a request registering concurrently is either swept or sees the mark (the same guarantee the shared lock gave). - **`ProcessRequestAsync` overrides:** a `ServerBase.ProcessRequestAsync` override now receives the server's stop token for endpoint requests instead of a token that is also linked to the transport. The request lifetime that services observe is still cancelled by the transport, the client and the timeout hint. - **Memory:** the transport pool can retain up to 64 MiB per bucket size after a peak, where `ArrayPool<byte>.Shared` would have trimmed it under memory pressure. There is no trimming yet. ## Testing - New tests: - `BoundedArrayPoolTests`: bucket sizing, reuse, bound under concurrent returns, a thousand outstanding buffers, a deep pipeline through `FastBufferManager` without allocation. - `ConcurrentLeasesStayWithinLimitsAndAreAllReturnedAsync`: 8 threads × 20k operations with ordinary, bootstrap and reconnect owners and a 4-entry owner table. A mutation that lets the capacity check overshoot by one fails it with 6,700 violations. - `FairRequestQueueTests`: concurrent selection in weighted mode and not in FIFO mode; relink in admission order with two threads (fails if relinked at the head); stop during grant. - `DrainsRacingLockFreeRegistrationAndCompletionAlwaysCompleteAsync`: 200 drains against four threads registering and completing requests and validation scopes. It times out if completions skip drains. - Suites (net10.0): Opc.Ua.Core.Tests Stack.*, Opc.Ua.Server.Tests, Opc.Ua.Sessions.Tests, Redundancy Server/Client/Samples, Stress Contract + Integration: | Suite | Passed | Failed | Skipped | |---|---|---|---| | Opc.Ua.Core.Tests `Stack.*` | 3604 | 0 | 80 | | Opc.Ua.Server.Tests | 8447 | 0 | 5 | | Opc.Ua.Sessions.Tests | 852 | 0 | 379 | | Opc.Ua.Redundancy.Server.Tests | 884 | 0 | 0 | | Opc.Ua.Redundancy.Client.Tests | 134 | 0 | 0 | | Opc.Ua.Redundancy.Samples.Tests | 17 | 0 | 0 | | Opc.Ua.Stress.Tests `Contract\|Integration` (Release) | 45 | 0 | 0 | - `dotnet format whitespace --verify-no-changes` on the changed files; net48 build. Not covered: `CustomNodeManager2`'s lock and the channel gate (separate roadmap items); admission sizing is in a parallel PR. Remaining per-request cancellation cost: the request lifetime links the server's stop token and the transport token in one source (240 B), so that shutdown and a closed channel still cancel running requests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-12 08:39:27 +00:00
romanettandClaude Opus 5.5	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
romanettandClaude Opus 5.5	9b034f8291	Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700) ... # Description Nullable reference types are now enabled once, in `common.props`, and every project in the repository builds warning-clean with them. **Configuration** - `<Nullable>enable</Nullable>` lives in `common.props`. All per-project `<Nullable>` settings are removed: 178 project files (enable, annotations, and the `disable` in the three Aot test projects). - 419 per-file `#nullable enable`/`restore` directives and the 4 `#nullable disable` directives are removed. - Kept on purpose: - The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`, `PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the project setting in generated code, so the directive is required there. - `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against the 1.5.378 packages and does not import the repository props. - `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now describe the central setting. **Migration** (the 43 unset and 13 annotations-only projects, roughly 10k diagnostics) - **Production code: annotated at the declarations.** Covers `tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the LDS sample. - Members, parameters and returns are nullable where null actually flows. - Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`, `IsMethodTypeDesign`). - `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take `params object?[]`. `Template.AddReplacement` takes `string?` (rendering already skips null replacements). - `IRegisteredServerStore.RegisterAsync` returns `Task<RegistrationEntry?>`: it returns null when a registration is removed. - Dereferences backed by an invariant (for example, the validated model) use `!`, so runtime behaviour is unchanged. - **Behavioural changes in production code** (small, deliberate): - `NodeSetToModelDesign` throws `InvalidOperationException` when the file is not a NodeSet. Before, it threw a NullReferenceException on the next line. - `ModelDesignValidator` starts its lookup tables empty instead of null. A redundant `if (m_nodes == null) return;` guard is gone. - **Test code: minimal annotations.** - `?` on values that can be null, `!` where the test asserts non-null. - `= null!` for fixture members assigned in SetUp. - `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the `IEncodeable` constraint. - A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`) folded into the adjacent `IsNull`/`Count` checks. **Notes for reviewers** - The diff is large (~1,500 files) but almost entirely mechanical: `?`, `!` and directive removal. The non-annotation changes are listed above. - Known follow-up, not done here: `IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay non-nullable even though the validator returns null before it loads a dictionary. Making them nullable would ripple through every generator call site. ## Related Issues - Fixes #4439 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. The "no new warnings" box covers net10.0 and net48: full non-incremental builds of `UA.slnx`, `tools/SourceGeneration.slnx`, `tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0 warnings and 0 errors under the repository's warnings-as-errors settings. net9.0 and net8.0 also had no nullable diagnostics, but they were not rebuilt after the final fixes. Tests have not been run locally yet. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-08 08:48:13 +02:00
3 people	f9892020a3	Add configurable server resource isolation (#4541) ... # Description Add configurable server resource isolation on top of the merged #4522 foundation. - Coordinate connection, handshake, retained-message, session-establishment, queue, execution, request-cost and parked-request limits through one server-scoped provider. - Keep configured startup, continuity/reconnect, control and trusted-owner reservations inside existing totals. Shared capacity remains usable up to explicit owner ceilings. - Use authoritative Session membership for reassembly continuity capacity without granting unrelated resource or rate privileges. Preserve explicit owner mappings and bounded tracking. - Cache immutable classification evidence while retaining current Session and classifier checks. - Support weighted request ordering or FIFO independently of resource accounting. Installed providers enforce decoded-request limits in both modes. - Preserve fail-fast reassembly, valid Abort handling, fixed deadlines, active-work protection, reverse-connect handoff and bounded OpenAPI processing. - Support custom-handler parking through `IRequestParkingPolicy`, `WithRequestParking(...)`, and `RequestLifetime.ParkSink`. Parking releases execution capacity only, keeping retained-data and parked charges until completion. Built-in Publish and the global opt-out remain unchanged. - Provide direct, fluent and DI configuration, public incoming-request parking support, effective startup limit logs and user-facing documentation. Balanced remains the default. SharedOnly without an explicit provider retains shared behavior for intentionally incompatible/unlimited configurations. Reservations are finite and are not one full-message allowance per Session. ## Related Issues Builds on merged #4522. Fixes #4554. ## Review and CI corrections All three previously missed findings in [review 5326734718](https://github.com/OPCFoundation/UA-.NETStandard/pull/4541#pullrequestreview-5326734718) are addressed in `9e25e4ecc`: 1. A custom FIFO provider receives RequestQueue, RequestQueueBytes, RequestExecution and ParkedRequest accounting. A blocked head cannot be overtaken. 2. `IResourceIsolationReassemblyProvider` replaces the sessionless threshold independently of `UseFairScheduling`. The aggregate byte ceiling remains enforced. 3. Runtime `ReservedReassemblyBytes` includes all configured protected stage floors, including ControlReserved. The review's two already-resolved changes remain intact. The CI collection race was fixed at its source in `e1a994a4b`: `ArrayOf.ToArrayOf` treats a separately read collection count as an initial capacity hint and grows/trims for the actual enumeration. Deterministic tests cover growth, shrinkage and stale-zero counts. The subsequent recovery-test race was fixed in `05753ce77`: fake time advances only after a gated keepalive response has been processed and reset its timer, not merely after request arrival. Recovery-order and timeout assertions remain intact, with additional response-state assertions. Instrumentation reproduced the original failure twice; both fixed variants passed 400 executions per framework. CodeQL alert 388 was verified and dismissed as a false positive. Startup logs contain aggregate numeric capacities, not credentials or caller identities. ## Validation | Scope | Result | | --- | --- | | Latest automated checks at `05753ce7775a58801b4ba9d8f6d7781b709d0b89` | 167 successful, one package-publication check skipped, no failures | | Remaining external prerequisite | `license/cla` pending: Contributor License Agreement is not signed yet | | FIFO/weighted queues, custom parking and server-channel selection | 175 passed on each net10.0/net48 | | Isolation, configuration and parking-hosting selection | 259 passed net10.0; 257 passed net48 | | ArrayOf conversion tests | 73 passed on each net10.0/net48 | | Recovery-audit and keepalive fixtures | 40 passed on each net10.0/net48 | | NativeAOT smoke from preceding updates | Published and executed successfully | No tests, assertions, coverage thresholds, or required checks were weakened. The remaining CLA prerequisite must be completed through [CLA Assistant](https://cla-assistant.io/OPCFoundation/UA-.NETStandard?pullRequest=4541). This is not CTT certification or an unconditional network-DDoS, whole-process-memory or fleet-wide guarantee. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [x] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-09-27 21:11:52 +02:00