Repository navigation
Server: remove the server-wide locks from the request pipeline, bounded transport buffer pool - #4742
Merged
Merged
Conversation
ArrayPool<byte>.Shared keeps a few arrays per bucket and processor. A server with about a thousand responses waiting to be sent exhausts it on a host with one or two processors (or a container limited to them), and then allocates a new 64 KiB chunk for every response: 54 KB per Read instead of 7.6 KB with 10 channels x 100 outstanding requests at DOTNET_PROCESSOR_COUNT=1. Buffer managers up to 1 MiB now rent from a BoundedArrayPool: power-of-two buckets in lock-free queues, up to 1024 arrays (64 MiB) in the largest bucket, one instance per maximum array length shared by all client and server managers of the process. Retained memory stays below 2 x 64 MiB. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every request takes three leases (RequestQueue, RequestQueueBytes, RequestExecution) and returns them, and each of the six calls took the provider's single lock: about 18 % of the server's on-CPU time spun on it with 10 channels x 100 outstanding Reads on 24 cores. Stage counters, owner usage and the protected floors are now reserved with bounded compare-and-swap loops that never pass a limit, and the counters already reserved are returned when 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 without any lock; a class count of an owner crosses zero only under a table lock, which keeps the exclusive class, the protected table slots and entry removal consistent with the leases. Lease release takes the table lock only for the owner's last lease of a class. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FairRequestQueue took its server-wide gate three times per request (enqueue, select, finish) and let only one worker select at a time: the others went back to wait for a wake signal while the selector ran the revalidation and the execution admission outside the gate. The selected head is now unlinked when it is selected, so an admitted dispatch needs no second gate entry, and in weighted mode several workers select at once. A head that finds execution capacity exhausted is relinked in admission order (concurrent selections of one owner may fail in any order) and its owner is not retried before capacity changes, as before. A request cancelled or a queue stopped while its admission runs is completed once by the selecting worker. Global FIFO mode still selects one request at a time, so its blocked head cannot be overtaken. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ServerBase.RequestQueue took its server-wide worker gate three to five times per request: in ScheduleIncomingRequest even in fair mode, where the locked block does nothing, in GrowFairWorkers at enqueue and at worker start, and twice in ProcessRequestSafeAsync to count running handlers. Fair mode now reads the stop flag without the gate (the fair queue rejects admissions itself once stopped). GrowFairWorkers checks the counters before it takes the gate, which it then needs only when a worker is to be added. Running handlers are counted with Interlocked; shutdown creates the drain completion after the workers have exited and the last handler completes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RequestManager took its server-wide lock four times per request (validation scope enter and exit, registration, completion) and wrote three AsyncLocal scopes, each set and restored. Executing requests and validation scopes are ConcurrentDictionaries, so registration and completion take no lock. A drain publishes a count before it snapshots them, and a completion takes the lock only when a drain is registered, so every completion a drain waits for still reaches it. The request timer is stopped only when no deadline was registered meanwhile. A Cancel remembers itself before it scans the registered requests, and registration ends with a full fence, so a request registering concurrently with a Cancel or a Session close sweep is either seen or sees the mark. The validation scope's AsyncLocal was never read and is gone; the request scope skips the ambient id when the dispatch scope already carries it. Enumeration, drains, timeouts and Cancel still run under the lock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The buffer-pool tests contain a nondeterministic allocation assertion and a contradictory memory-bound case.
2 open findings
What changed in this PR
Removes server-wide request-pipeline contention and introduces bounded, process-wide transport buffer pooling.
Changes:
- Reworks request admission, dispatch, draining, and shutdown accounting with concurrent collections and atomic operations.
- Adds a bounded transport buffer pool with updated sizing behavior.
- Adds concurrency, ordering, shutdown, and buffer-reuse tests.
| File | Description |
|---|---|
src/Opc.Ua.Core/Stack/Bindings/BoundedArrayPool.cs |
Implements bounded process-wide pooling. |
src/Opc.Ua.Core/Stack/Bindings/ArrayPoolBufferManagerBase.cs |
Selects the bounded pool and updates sizing. |
src/Opc.Ua.Core/Stack/Server/FairRequestQueue.cs |
Enables concurrent weighted selection and ordered relinking. |
src/Opc.Ua.Core/Stack/Server/ServerBase.RequestQueue.cs |
Reduces worker-gate locking and revises shutdown draining. |
src/Opc.Ua.Server/ResourceIsolation/DefaultServerResourceIsolationProvider.cs |
Replaces global admission locking with atomic accounting. |
src/Opc.Ua.Server/Server/RequestManager.cs |
Makes registration and completion lock-free. |
src/Opc.Ua.Server/Server/StandardServer.cs |
Updates request-registration synchronization documentation. |
tests/Opc.Ua.Core.Tests/Stack/Server/FairRequestQueueTests.cs |
Tests concurrent selection, ordering, and stopping. |
tests/Opc.Ua.Core.Tests/Types/Bindings/ArrayPoolBufferManagerBaseCoverageTests.cs |
Updates pool-selection and sizing coverage. |
tests/Opc.Ua.Core.Tests/Types/Bindings/BoundedArrayPoolTests.cs |
Tests bounded pooling and reuse. |
tests/Opc.Ua.Server.Tests/RequestManagerTests.cs |
Stress-tests concurrent drains. |
tests/Opc.Ua.Server.Tests/ResourceIsolation/RuntimeResourceIsolationTests.cs |
Stress-tests concurrent admission limits. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
marcschier
approved these changes
Oct 12, 2026
Code coverage✅ Coverage gate passed.
Uncovered changed lines
Coverage is above the recorded baseline - consider ratcheting Thresholds live in |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4742 +/- ##
==========================================
- Coverage 82.86% 82.80% -0.06%
==========================================
Files 2268 2300 +32
Lines 346319 351096 +4777
Branches 60386 61265 +879
==========================================
+ Hits 286985 290738 +3753
- Misses 39233 39881 +648
- Partials 20101 20477 +376
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…en once Per request, EndpointIncomingRequest.CallAsync built a CancellationToken[] and a separate timeout CancellationTokenSource for the TimeoutHint, and linked both with the caller's and the transport's token (528 bytes for the lifetime alone). Before that, the request queue had already linked the worker's stop token with the same transport token. The new RequestLifetime(TimeSpan timeout, CancellationToken first, CancellationToken second) constructor links the two tokens (only those that can be cancelled, a duplicate once) and applies the timeout with CancelAfter on the same source: 240 bytes. A timeout still leaves the status Good, which CallAsync reports as Bad_Timeout. Requests that link their scheduled token themselves say so through the internal ILinksScheduledCancellation, and the queue then passes the stop token alone instead of a second linked source. Other IEndpointIncomingRequest implementations keep the linked token. A ServerBase.ProcessRequestAsync override now sees the server's stop token for endpoint requests; the request lifetime the services observe is still cancelled by the transport, the client and the timeout hint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review feedback: the default pool kept at least 16 arrays, so a pool for arrays above 4 MiB could retain more than the documented 64 MiB per bucket. The floor is now one array. The transport only uses the pool up to 1 MiB, where the bound was already 64 arrays, so nothing changes for it. The deep-pipeline test measured allocations through the process-wide pool, which parallel tests share; it now uses a private pool with the default configuration and checks that every buffer is reused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 of 7 tasks
marcschier
enabled auto-merge (squash)
October 12, 2026 08:20
romanett
added a commit
that referenced
this pull request
Oct 12, 2026
…that are actually reused, ready list instead of an O(N) scan (#4748) Roadmap item A5 of the o6 benchmark gap analysis. Every Publish response deep-cloned its NotificationMessage in `SentMessageQueue` (PR #4707 kept the clone because a retained message returns its pooled payload when the client acknowledges it, which can happen before the response was sent). This PR replaces the clone with a send-completion lease, makes the activator pool that recycles the notifications O(1) and sizes it for the server's load, builds messages without intermediate copies (the notification array is rented), and lets monitored items tell their subscription when they have something to report so the publish timer no longer walks every item. ## Results Paired A/B against master `0dff94bd3` (includes #4742 and #4743; alternating builds, 3 rounds, medians; 20 sessions unless noted, workstation GC, 24-core machine shared with other sessions). Server = the reference server child process. | Scenario | Server alloc / op | Server GC pause | Server CPU / op | Write→notify p50 / p99 | Throughput | |---|---|---|---|---|---| | `sub`, 5 subs × 100 items, pub 100 ms | **−57 %** (373 → 159 B) | **−57 %** | −7 % ¹ | −1 % / −2 % | +1 % ² | | `sub`, 5 subs × 100 items, pub 10 ms | **−57 %** (375 → 163 B) | **−52 %** | +9 % ¹ | −22 % / +1 % | ±0 ² | | `sub`, pub 100 ms, SignAndEncrypt | **−57 %** (380 → 163 B) | **−53 %** | +4 % ¹ | −11 % / **−12 %** | ±0 ² | | `sub`, 2 sessions × 1 sub × 1000 items, 20 changing, pub 50 ms (9 rounds) | −2 % ³ | ±0 | −23 % ³ | ±0 / −2 % | ±0 | | `counters`, 4000 items, 10 ms sampling, pub 1 s (per notification) | −2 % | −6 % | −6 % | – | ±0 | | `o6counters`, 2048 items (per sample) | −3 % ⁴ (111 → 108 B) | −5 % | −7 % | – | ±0, all PASS | | `o6counters`, 4096 items (per sample) | −3 % ⁴ | +14 % ⁵ | −6 % | – | ±0, all PASS | ¹ CPU per notification varies by ±15 % between rounds on this machine; in an earlier 8-round batch against `071660354` the same scenarios measured −14 % / −1 %. Read it as "no regression". Client CPU per notification moved −18 %…0 %, within its spread. ² Bounded by the writer rate. ³ The server is nearly idle (0.05–0.12 cores, 360 notifications/s; its allocation is the writer and keep-alives). The first 3 rounds measured +31 % CPU, 6 more rounds −23 % (medians 969 → 742 ms per window): noise-bound, no regression. ⁴ With #4743's sampling permission cache a sample costs 111 B; one Publish per second carries 2048/4096 notifications. ⁵ The activator pools no longer allocate their 1023-slot arrays up front; the heap is a few MB smaller, which shifts the GC's generation budgets so that some low-allocation runs make every collection gen1 (26 of 26 instead of 2). Pause totals differ by milliseconds per 10 s window (37 → 42 ms here, 21 → 20 ms at 2048 items). Allocating the arrays eagerly restored master's gen1 counts in a separate test. In a single CPU trace of `sub` at pub 10 ms, recycling acknowledged payloads (pool `Return`) went from 16 % of server on-CPU time (master, the full pool scanned per return) to 5 %. ## Changes - **Send completion instead of a deep clone** (`NotificationMessageLease`, `LeasedPublishResponse`, `SentMessageQueue`): the retransmission queue and every Publish response that carries a message each own a reference. The transports already call `IPooledEncodeable.Reuse()` on a response once it is encoded or dropped; the Publish response is now a `LeasedPublishResponse` whose `Reuse()` releases its reference. The pooled payload (`NotificationMessage`, `DataChangeNotification`/`EventNotificationList`, `MonitoredItemNotification`/`EventFieldList`) is recycled when the last owner releases it, so an acknowledgement that overtakes the send no longer recycles a payload that is still being encoded, and the queue and the response share one message. A reference that is never released only leaves the payload to the GC; every owner releases at most once. Keep-alive messages are owned by their response alone. Republish, the durable snapshot and the retransmission mirror still copy. - **Transports**: the HTTPS (binary and JSON), WebSocket (UA-SC, JSON, OpenAPI) and Kestrel TCP listeners and the WebAPI endpoint dispatcher now release responses after encoding, as the TCP listener does, so leased payloads are recycled there too. A response retained for a reconnect is released when it is sent; a send that throws leaves the response to the GC. - **Activator pool** (`BoundedObjectPool`): rent and return scanned the 1023 slots linearly; with the pool full every returned notification scanned all of them before being dropped (16 % of server CPU at 94k notifications/s). It is now a `ConcurrentQueue` behind the lock-free first slot with an `Interlocked` bound (a locked stack spun under contention when transport threads return whole messages). The queue is created on first use, so activators of types that are never recycled no longer allocate a 1023-slot array each. The client decoder rents through the same pool. - **Pool bound** (`PooledEncodeableType<T>.EnsureMaximumRetained`, internal): the bound can be raised, never lowered. The subscriptions raise the `MonitoredItemNotification` and `EventFieldList` pools to twice the number of monitored items in the process (a notification is recycled about two publishing cycles after it was built), between the default 1024 and 65536. At the cap the retained notifications take about 5 MB; the bound stays raised after items are deleted. - **Message construction** (`Subscription.InnerPublish`/`ConstructMessage`, `MonitoredItem.Publish`): the per-Publish `Queue<>`/`List<>` objects are reused by the subscription; messages are built at their final size (no `List`→`ArrayOf` copy, no `AddItem` reallocation); the `MonitoredItemNotification[]` of a message is rented from `ArrayPool` and the message carries an exact-length slice of it; the lease returns the array (cleared) with the payload, and messages dropped on queue overflow return theirs at once; only arrays the subscription rented are tracked, so restored, mirrored and copied messages never reach the array pool; the `DiagnosticInfo` list is only allocated when a notification has one; timestamps are stripped on the struct before the pooled notification is filled. - **Ready list** (`IReadySignalingMonitoredItem`, `Subscription.PromoteReadyMonitoredItems`): `MonitoredItem` acquires a ready signal (compare-exchange, a full fence after its flags are set) when it queues a value or event or a resend is requested, and hands itself to the subscription's intake list. The publish timer checks only these candidates instead of every item under the subscription lock. An item stays a candidate while it has something pending that can become reportable without a new value (sampling deadline, aggregate interval, samples of a SAMPLING item, trigger flag); after releasing the signal the subscription checks again, so a value queued concurrently is never missed. Created, restored, modified, monitoring-mode and triggering changes mark candidates, as does an item that leaves the publish list with something pending. ### Behaviour notes (custom subscription or monitored-item extensions) - **Publish responses share the retained message.** `PublishResponse.NotificationMessage` returned by `SubscriptionManager.PublishAsync` is the retained message itself, not a copy, and its payload objects return to the activator pools after the response was sent and acknowledged. Code that keeps or modifies the message of a Publish response (for example in a `StandardServer.PublishAsync` override or a request interceptor) must copy it first (`CoreUtils.Clone`). `Republish` still returns a copy. - **Custom transports**: a transport that does not call `Reuse()` on responses keeps working; the payload is then collected by the GC instead of being pooled. - **Custom `IMonitoredItem` implementations** (not derived from `MonitoredItem`) cannot signal readiness: a subscription that contains one scans all its items on every publishing cycle, as before. `MonitoredItem` subclasses signal through the base class. - **`DataChangeNotification.MonitoredItems` of a published message is a slice of a pooled array**: read it through `ArrayOf<T>` (`Span`, indexer, `Count`); code that unwraps the underlying array would see a longer array. - **Order within a message**: notifications of different monitored items appear in the order the items became ready rather than in the internal list order. OPC UA does not define an order across items; the order of the values of one item is unchanged. ## Harness (`perf/`) - `--write-nodes K` for `sub`: the writer changes only the first K monitored nodes, for large subscriptions in which few items change. - `server-scenarios.txt`: publish-path scenarios (`--pub 10`, 1000 items per subscription with 20 changing). ## Testing - New `NotificationEncodingLifetimeRegressionTests` cases: an acknowledgement before send completion does not recycle the payload still being encoded (the send completion does); Republish after the send completed returns the original content and stays independent of the recycled original; a failed send that releases the response twice, plus a copy of the response, recycles the payload exactly once and only after the queue released it; a keep-alive is owned by its response; the rented notification array stays intact until both acknowledgement and send completion (either order), the message encodes only its slice, and queue overflow returns the arrays of dropped messages. The existing 30-case matrix (publish / queued / republish / mirrors × acknowledge / evict / clear × data / events) now asserts that a shared payload survives every queue mutation until its response is released. - New `ReadyListRegressionTests`: the same scenarios on a subscription that uses the ready list and on one that scans (it also has an item that cannot signal) must publish the same notifications, for triggering (sampling items, backlog of the triggering item), monitoring-mode changes (Sampling↔Reporting, Disabled, disabled before publish), disabled publishing with keep-alives and ResendData, a value queued while the item was in the publish list, and the backlog of an item that left Reporting while in the publish list. Mutation check: removing the item's signal fails 5 of 6, removing the subscription-side marks fails the backlog case. Plus: a published item without pending values is not tracked; creating 1200 items raises the notification pool bounds to at least 2400. - `PooledEncodeableTypeTests`: the raised bound is respected and never lowered. - net10.0 on the rebased branch: Server 8481 passed (5 skipped); Sessions 851 (379 skipped); Client 2788; Subscriptions.Classic 57; Subscriptions.Durable 36 (11 skipped); Redundancy.Server 884, Redundancy.Client 134, Redundancy.Samples 17; Stress Contract+Integration 45; Subscriptions 657 (6 skipped); Types pool tests 165. All 0 failed. - net48 build of `Opc.Ua.Server.Tests` and `Opc.Ua.Bindings.Https` and `Opc.Ua.Types.Tests` clean; `dotnet format whitespace` clean. ## Not changed (follow-ups) - Republish still copies the message (`ISubscriptionManager.Republish` returns a `NotificationMessage`, so the response cannot carry a lease without an API change). - `EventFieldList[]` per event message could be rented like the data change array. - Sampling remains the larger share of allocation and CPU in the `counters`/`o6counters` scenarios. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
3 of 7 tasks
1 of 7 tasks
romanett
added a commit
that referenced
this pull request
Oct 13, 2026
…lidation (#4753) # Description Every request passes through `SessionManager.TryGetSessionContext` at admission and again at dispatch (`FairRequestQueue.GetRevalidationStatus` → `DefaultServerResourceIsolationProvider` → `ISessionBindingProvider.TryGetSessionContext`), and then through `Session.ValidateRequest`. Together they took: 1. the server-wide `m_bindingsLock`, to confirm the candidate binding was still the committed one; 2. the per-session diagnostics lock in `Session.HasExpired`. It is inlined, so CPU traces charge its spin to `TryGetSessionContext`; 3. the session lock in `Session.IsSecureChannelValid`; 4. the session lock around all of `Session.ValidateRequest`, plus the diagnostics lock twice (contact refresh, diagnostic counters); 5. the server-wide `SessionManager` event lock, to raise `SessionDiagnosticsChanged` for every request even when nothing subscribes. With the FairRequestQueue lock also removed (a local change without the queue-wide lock, not yet a PR), these were the next waiting points: 14–16.5 % of on-CPU samples spun in `Lock.TryEnterSlow` under `TryGetSessionContext`, and once that was fixed another 7–10 % under `Session.ValidateRequest`. ## Changes **SessionManager: confirm the binding without the bindings lock** - `SessionActivationState` publishes a `DispatchBinding` (the committed binding while no activation is committing, otherwise `null`) with `Volatile.Write`. The `BindingContext` and `IsCommitting` setters republish it, and every writer of those already runs under `m_bindingsLock`. - The lookup reads it once for the candidate and once to confirm. A commit always creates a new `SessionBindingContext`, so an unchanged reference means nothing replaced it. A token is mapped to a session object at most once and is never re-added after removal, so finding the token still mapped to the same session afterwards means both held at the instant of the confirming read, which is the guarantee the lock gave. - IsCommitting, transfer, close/claim, shutdown and the certificate/security checks are unchanged. **Session: read the channel id without the session lock** - `SecureChannelId` is published with a volatile write in `Activate`, and `IsSecureChannelValid` reads it without the session lock. **Session: request validation and diagnostic counters without locks** - Request counts are interlocked increments of per-session slots (total and error per `RequestType`, plus unauthorized). They are applied to `SessionDiagnostics` under the diagnostics lock whenever it is read (`ReadDiagnostics`, `UpdateDiagnostics`, the diagnostics-node callback, `ClientLastContactTime`), so readers see the same totals. - The contact refresh is a compare-and-swap on the tick count, and requests within the same millisecond do not write. An expiry observed by `HasExpired` or by a refresh is recorded in the contact word (`kExpiredFlag`), so a request that read the clock before the monitor saw the timeout cannot refresh it afterwards. This keeps the documented "a request cannot revive a session the monitor saw expire" guarantee without the lock. The intermediate commit that made `HasExpired` lock-free on its own had opened that window; this commit closes it. - `ValidateRequest` takes no session lock: the channel id and `Activated` are published with volatile writes. - `RaiseSessionEvent` returns before taking the event lock when a `DiagnosticsChanged` event has no subscriber. Behaviour differences reviewers should know about: - `ClientLastContactTime` is millisecond-granular, and two concurrent refreshes can publish their wall-clock times in reverse order, which leaves it up to about 1 ms older than the latest contact. - After an expiry has been observed, an `Activate` no longer refreshes the contact time. `SessionManager` already rejects activation of an expired session before that point. ## Tests - `LookupDuringActivationCommitReturnsNoBindingAsync`: a lookup inside the commit window (after `Session.Activate`, before the binding commit) returns no binding. A test session re-implements `ISession.Activate` to run the hook. Before this test, publishing the binding regardless of `IsCommitting` passed all 690 session tests. - `LookupRacingTransfersAndCloseReturnsOnlyCommittedBindingsAsync`: 4 readers look up while 1000 transfers alternate between two channels, followed by a close. Each result must be a committed binding of the queried channel, no older than the last completed activation and never decreasing, and nothing may come back after the close. - `RequestRacingObservedExpiryDoesNotReviveTheSessionAsync`: a clock hook lets the monitor observe the expiry between a request's clock read and its refresh, and the request must fail with `Bad_SessionClosed`. It fails if `HasExpired` does not record the expiry. - `ConcurrentRequestsAreCountedExactlyAsync`: 8 threads × 2100 validations (Read/Browse, plus rejected Writes). The totals, per-service counts, error counts and unauthorized count must be exact, and a concurrent reader never sees the total go backwards. - Local runs (net10.0): `Opc.Ua.Server.Tests` 8487 passed / 0 failed; `Opc.Ua.Sessions.Tests` 851 passed / 0 failed / 379 skipped. net48 builds. ## Measurements `perf/ServerLoadHarness`, Server GC, paired alternating A/B, 7 rounds. Baseline is master 8b360da + that FairRequestQueue change; candidate adds the three commits. | scenario | CPU µs/req | alloc B/req | lock contentions/req | req/s | |---|---|---|---|---| | read 1 node, 10 sessions × 100 in flight | 25.4 → 24.1 (−5 %) | 5456 → 5455 | −66 % | +1 % | | read 1 node, 200 sessions | 42.9 → 43.7 (+2 %) | 5225 → 5212 | −60 % | −5 % | CPU and throughput differences are within this shared machine's run-to-run noise (CPU ranges overlap on both sides). Allocation and contention per request are the stable signals. CPU trace (10 × 100), share of on-CPU samples spinning in `Lock.TryEnterSlow`, 2 runs each: | build | total | under `TryGetSessionContext` | under `Session.ValidateRequest` | |---|---|---|---| | baseline | 20.8 / 23.7 % | 14.0 / 16.5 % | 0.8 / 0.9 % | | + binding snapshot | 18.0 / 16.6 % | 12.5 / 11.3 % (inlined `HasExpired`) | 0.9 / 0.7 % | | + lock-free `HasExpired` / channel id | 9.5–13.5 % | 0 | 4.8–9.9 % | | + lock-free validation and counters | 3.5 / 3.4 % | 0 | 0 | The spin moves rather than disappearing: `Monitor.Enter_Slowpath`, from the channel locks in `TcpServerChannel.HandleIncomingMessageAsync` / `SendResponseAsync` and `ChannelGate`, rises from 37–40 % to about 47 %. That is the next waiting point. On master alone (FairRequestQueue lock still in place), CPU/req and alloc/req changes are within noise. ## Related Issues - Follow-up to the o6 benchmark gap work (#4742, #4743). ## 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. - [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. (Opc.Ua.Server.Tests and Opc.Ua.Sessions.Tests on net10.0; net48 compiles) - [ ] 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>
romanett
added a commit
that referenced
this pull request
Oct 14, 2026
… request path (−16 % allocation per Read) (#4767) ## Description Five small changes to the per-request server path, found in the CPU and allocation profiles of the o6 read benchmark (`AsyncCustomNodeManager`, Server GC, 10 clients × 100 Reads in flight) on master 3a31473. Each change is its own commit and can be reviewed and reverted on its own. The channel gate (about 40 % of waits) is not touched; that is A1. ### Results Paired A/B runs against master (0e40e12), alternating builds per round, 5 rounds, medians (min..max). The machine is shared with other sessions: allocated bytes per request are deterministic; CPU per request is best read from the runs pinned to 4 cores. | Scenario | master | this PR | Change | |---|---|---|---| | o6read 10 × 100, server pinned to 4 cores: CPU / request | 39.1 µs (35.9..39.8) | 36.9 µs (35.5..37.6) | **−5.6 %** | | same: allocated / request | 6,198 B | 5,224 B | **−15.7 %** | | same: req/s | 98.7k (95.4..108.5k) | 104.6k (97.6..105.9k) | +5.9 % | | o6read 10 × 100, 24 cores: req/s | 183.7k (173.3..198.0k) | 189.8k (178.9..197.5k) | +3.4 % | | same: allocated / request | 6,500 B | 5,578 B | −14.2 % | | o6read 10 × 100 Basic256Sha256, 24 cores: req/s | 159.8k (149.4..178.7k) | 164.2k (156.7..165.3k) | +2.7 % | | same: allocated / request | 6,590 B | 5,694 B | −13.6 % | | `read --nodes 1`, 20 sessions: req/s | 155.3k (141.6..157.6k) | 163.8k (163.4..166.5k) | +5.5 % | | same: allocated / request | 5,548 B | 4,689 B | −15.5 % | | `read --nodes 100`: allocated / request | 74.4 KB | 70.1 KB | −5.8 % | | `browse`: allocated / request | 39.8 KB | 35.0 KB | −12.2 % | | `browse`: lock contentions / 1000 requests | 474 | 367 | −23 % | At 24 cores CPU per request stays within noise, because the time these changes free now waits at the channel gate (see below). Per item, pinned to 4 cores (5 rounds; CPU deltas of one item are within run-to-run noise, allocation deltas are not): | Item | CPU / request | Allocated / request | |---|---|---| | Q1 system context | +1.8 % | −1.7 % | | Q2 request table | −4.3 % | −1.4 % | | Q3 request timeouts | −2.8 % | −2.2 % | | Q4 namespace metadata | −3.5 % | ±0 | | Q5 read-path allocations | −2.4 % | −10.9 % | Sampled thread time at 24 cores (cpu trace, first `Opc.Ua` frame above the wait or copy): | Frame | master | this PR | |---|---|---| | `ServerSystemContext.Copy` (incl. `MemberwiseClone`) | 12.4 % | 0.1 % | | `RequestManager` (`ConcurrentDictionary` inside `EnterValidationScope`, scope `Dispose`, `RequestReceived`, `CompleteRequest`) | 7.1 % | 0 | | `RequestLifetime` (CTS registration spin, ctor/`RegisterExternal`/`DisposeResources`) | 4.8 % | 0.2 % | | `NamespaceMetadataRegistry.GetAsync` lock | 4.0 % | 0 | | `RuntimeHelpers.InitializeArray` (`ReadValidationAttributes`) | 2.2 % | 0 | | Channel gate (`ChannelGate+AwaitEntryAsync`, inclusive) | 31 % | 46 % | ## Changes **Q1: one system context copy per request and node manager.** `AsyncCustomNodeManager`'s service methods (Read, Write, Browse, TranslateBrowsePaths, Call, HistoryRead/Update, monitored item and event services) use the per-request copy from `OperationContext.GetSystemContext` instead of copying the node manager's context again. The per-request cache keeps the first node manager's copy without an extra object and up to eight more, so a request that touches several node managers no longer evicts the other one's copy. `ServerSystemContext` copies itself with a copy constructor instead of `MemberwiseClone` (which allocates through a runtime call) unless it is a derived type; `SessionSystemContext` gains a protected copy constructor. Copies per Read went from about three to one. **Q2: lock-free request registration in `RequestManager`.** #4742 took the server-wide lock off the request path, but the two `ConcurrentDictionary`s for executing requests and open validation scopes stayed the main waiting point after the channel gate (four writes per request from every worker). `RequestTable<T>` places an entry with a compare-and-swap into a free slot near a scrambled hash of its key, and the entry remembers its slot so completion is one more compare-and-swap; only when every probed slot is taken does an entry go to an overflow set under a lock. Drains, Cancel, Session close, the timeout sweep and `Dispose` read the slots one after the other, which keeps the existing ordering arguments (registration fence vs. Session/Cancel marks, drain count published before the snapshot). Sharding the dictionaries over per-processor locks was tried first and only moved about 5 % of waits into the shard locks. **Q3: no timer or shared-token registrations per request.** `RequestLifetime`'s timeout constructor, used by the endpoint for every request, no longer creates a `Timer` and no longer registers with the server's request-queue token and the transport token, which every worker registered with and so contended on. The deadline and the two tokens are watched by `RequestDeadlineMonitor`: one sweep every 20 ms over intrusive lists sharded by processor, running only while requests are monitored. The `params` constructor keeps its synchronous registrations. **Q4: namespace metadata lookups without a lock.** `NamespaceMetadataRegistry` reads its caches (found and missing, by URI and now also by index) without its lock; only filling and invalidating take it. The generation moves before the caches are cleared, so a lookup racing an invalidation never reports a stale miss. **Q5: fewer per-call allocations on the read path.** `ReadValidationAttributes` fills its attribute-id array element by element instead of with an array initializer (`RuntimeHelpers.InitializeArray` per call); single-node Read and Write requests skip the per-request attribute cache and validate with `permissionsOnly` (the same three attributes, read once); `ReadAsync` allocates the list of nodes to validate only when needed. (Review: a shared static attribute array, a per-thread `Variant[]` scratch array and a per-thread Read node cache were dropped again because they crossed virtual calls; the measurements below were taken with them, so Q5's allocation saving is a little smaller than shown.) ## Behaviour notes - **Request timeouts (Q3):** a `TimeoutHint` now fires up to 20 ms late (the sweep period), and cancelling the server's or the transport's token reaches the request's `CancellationToken` up to 20 ms later instead of synchronously. A token that is already cancelled cancels the request at once, and `TryCancel` checks the tokens first, so a token cancelled before an explicit Cancel still owns the terminal state. The exactly-once terminal state of #4744 is unchanged: the sweep claims it through the same compare-and-swap. - **System contexts (Q1):** a custom node manager that changed fields of the context it got in a service call now changes the copy shared by that request's other calls into the same node manager; it never reaches other requests or the node manager's own `SystemContext`. - **Request ids (Q2):** `RequestManager.RequestReceived` no longer throws for a different `OperationContext` with a request id that is still registered. Requests are tracked by their context; ids come from a server-wide counter. ## Testing - New: `ServerSystemContextCopyTests` (every field copied, derived types kept), `AsyncCustomNodeManagerTests` (one copy per node manager and request, two managers in one request, not shared across requests; single- vs multi-node Read verdicts identical incl. a node read twice), `RequestTableTests` (idempotent add, exactly-once remove, overflow, concurrent churn while enumerating; drain racing completion on many threads; Session close and Cancel by handle exactly once while others register), `RequestDeadlineMonitorTests` (timeout precision with a fake clock, token propagation, ownership vs. explicit Cancel, no leaked entries, sweep/cancel/complete race), `NamespaceMetadataRegistryTests` (cached miss by index invalidated, lookups racing `Invalidate`, permission changes visible through the cached node). - Run locally (net10.0): Opc.Ua.Types.Tests, Opc.Ua.Core.Tests (Stack), Opc.Ua.Server.Tests, Opc.Ua.Sessions.Tests, Opc.Ua.Subscriptions.Tests, Opc.Ua.Redundancy.Server.Tests, Opc.Ua.Stress.Tests Contract + Integration. net48 builds. - Measured with `perf/ServerLoadHarness` scenarios `o6read` (o6 harness, not part of this PR), `read`, `browse`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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/Lockslow 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, becauseArrayPool<byte>.Sharedruns out with about a thousand responses in flight.No wire or configuration changes. The only public API change is an additive
RequestLifetimeconstructor. 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(f5062be): 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.DOTNET_PROCESSOR_COUNT=1)Contention (
--trace-profile contention, o6read 10×100, 24 cores, one run per side): the isolation provider,FairRequestQueue, the request queue's worker gate andRequestManagerwent from 26 % of all contention events (~42k in the window) to 1.2 % (~2k), at 40 % higher throughput. What remains isCustomNodeManager2's lock and the dispatcher's permission path (separate roadmap items). In the sampled CPU profile,Lock.TryEnterSlowfell 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 50p99 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-freeConcurrentQueues, at most 1024 arrays and at most 64 MiB in the largest bucket (1024 × 64 KiB for the defaultMaxBufferSize), retained memory below 2 × 64 MiB. Arrays are retained only once returned, so a pool grows to the peak concurrency it has seen.ArrayPoolBufferManagerBaseuses it instead ofArrayPool<byte>.Sharedfor buffer sizes up to 1 MiB (larger sizes keep their configurable pool).GetExpectedBufferSizeno longer doubles for the next bucket on .NET Framework, since the bounded pool allocates the exact bucket when it is empty.Isolation provider (#5)
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.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)
ServerBase.RequestQueue (#5)
ScheduleIncomingRequest.GrowFairWorkerspre-checks the counters and takes the gate only to add a worker.Interlocked; shutdown's drain is created after the workers exit and completed by the last handler.One cancellation source per request
RequestLifetime(TimeSpan timeout, CancellationToken first, CancellationToken second). It links only the tokens that can be cancelled (a duplicate token once) and applies the timeout withCancelAfteron the same source.EndpointIncomingRequest.CallAsyncuses it instead of aCancellationToken[], a separate TimeoutHint source and a linked source over three tokens. The lifetime drops from 528 B to 240 B, and a timeout still reportsBad_Timeout.ServerBase.RequestQueueno longer links the transport token again for requests that link it themselves (internalILinksScheduledCancellation, implemented byEndpointIncomingRequest). OtherIEndpointIncomingRequestimplementations keep the linked token.--nodes 1 --inflight 506,446 → 6,164 B); CPU and throughput unchanged within noise.RequestManager (#5)
ConcurrentDictionarys; registration and completion take no lock.AsyncLocalwas write-only and is removed. The request scope skips the ambient id when the dispatch scope already carries it: 4 fewerExecutionContextallocations per request.Behaviour notes
Bad_RequestCancelledByClient/Bad_ServerHalted), as before.ProcessRequestAsyncoverrides: aServerBase.ProcessRequestAsyncoverride 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.ArrayPool<byte>.Sharedwould 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 throughFastBufferManagerwithout 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:
Stack.*Contract|Integration(Release)dotnet format whitespace --verify-no-changeson 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