4 Commits

Author	SHA1	Message	Date
romanett	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
romanett	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
Marc Schier	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
Marc Schier	7cec2d614d	Move repository files to the consolidated layout	2026-07-16 06:25:54 +02:00