mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-27 04:59:55 +08:00
master
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 romanettandClaude Opus 5.5
|
5f6faa60d7 |
Server: fewer allocations per single-value Read (isolation reservations, scope reuse, routing capture without restore) (#4856)
Removes per-request allocations on the server's single-value Read path
with small, internal changes. No wire-format change, no public API
change, no change of the execution model (the receive loop is
untouched). Exactly-once semantics of the isolation leases,
cancellation, timeouts, diagnostics and auditing are unchanged.
## Effect
Allocated bytes per single-value Read (perf harness, `native-read`, 10
clients x 100 in flight, `AsyncCustomNodeManager`, Server GC;
deterministic):
| | 1 core | 4 cores |
|---|---|---|
| master
|
||
|
Marc Schier
|
9bfc780e14 |
Update NuGet dependencies and migrate tests to NUnit 5 (#4840)
# Description Update 55 NuGet dependencies to the latest stable releases available on 2026-10-19, preserving documented compatibility pins and framework-specific version bands. Existing OpenUSD prereleases remain unchanged because no newer releases are available. Migrate existing tests to NUnit 5: await asynchronous assertions, propagate async through helpers, handle nullable predicates, and replace obsolete `Order` attributes with sequencing dependencies. Sequencing dependencies use `AllowFailure = true`; filtering a dependent test also runs prerequisites. Adjust fixture-level parallelism where NUnit dependency scheduling requires it. Keep Roslyn API/runtime pins intact; select newer Immutable/Metadata runtime packages only for test and profiling hosts requiring them. Update the approved SDK pin to 10.0.402 across local, CI, and container builds. Document NUnit 5 conventions. |
||
|
romanettandClaude Opus 5.5
|
33d6d29e12 |
Server: wake fair queue workers through a LIFO signal, let a looking worker take new requests (#4833)
Request queue workers of the `FairRequestQueue` waited on a bounded `Channel<bool>(1)` for the coalesced wake token. With many cores, its monitor became the server's largest lock contention point, and each wake cost a channel operation, an allocated wait and a thread-pool hand-off. This PR replaces that signal and lets a worker that just finished a request keep looking for the next one for a few microseconds before it parks. The concept is unchanged: - requests are admitted into the fair queue; - `MinRequestThreadCount`/`MaxRequestThreadCount` workers dequeue and run them; - the receive loop is not touched; - weighted fairness, the per-owner and global limits, leases, admission and `BadServerTooBusy`, cancellation with an exactly-once lease release, stop and drain, and parked publish requests behave as before. There are no wire, public API or configuration changes. ## Results Paired A/B against `master` |
||
|
romanettandClaude Opus 5.5
|
19a97da52e |
Server: confirm unchanged isolation classifications at dispatch without the full lookup (#4787)
Roadmap item R3 of the o6 benchmark gap analysis (master `5b03e9afa` profile). Each request was revalidated twice by the resource-isolation provider: at dequeue, and again before execution. Each revalidation repeated: - the owner table lookup (a `ConditionalWeakTable`); - the session binding lookup (token and session dictionaries, the session's expiry clock); - the classifier; - the owner key checks. Now the classification also returns the evidence it was derived from. The queue confirms that evidence at dispatch and runs the full revalidation only when it no longer holds. No wire, configuration or public API change. Every rejection status code, and the order of the checks relative to cancellation and session close, is unchanged: the confirmation only ever short-circuits to Good, and the full revalidation decides every other outcome. ## Results Paired A/B against master `af601ebca` (= `5b03e9afa` + #4778). The builds alternate per round, with the order rotated each round, on the same machine. The table shows medians of 5 rounds, with the throughput range of the rounds in brackets. The machine was shared with other work; allocated bytes per request are deterministic. - **o6read** uses the native open62541 client workers of the o6-automation benchmark, in the o6 2.0.1 configuration: `AsyncCustomNodeManager`, Server GC, queue = clients × outstanding + 3. - **"4 cores"** pins the server to 4 cores (`--server-affinity 0xF`, `DOTNET_PROCESSOR_COUNT=4`). - **The other rows** use the harness's .NET client, 20 sessions unless noted. | Scenario | req/s | Server CPU µs/req | Alloc B/req | |---|---|---|---| | o6read 10×100, 4 cores | 130,300 → 135,400 (+4 %) [129–135k → 132–140k] | 30.3 → 28.6 (**−6 %**) | 4,706 → 4,709 | | o6read 10×100 encrypted, 4 cores | 116,300 → 122,500 (**+5 %**) [113–119k → 118–126k] | 33.4 → 31.5 (**−6 %**) | 4,747 → 4,756 | | o6read 10×100, 24 cores | 344,500 → 347,400 (+1 %) [328–350k → 331–362k] | 53.6 → 52.2 (−3 %) | 4,710 → 4,716 | | o6read 10×1000, 24 cores | 310,800 → 307,300 (−1 %) [297–332k → 307–323k] | 55.6 → 57.5 (+4 %, noise) | 4,719 → 4,730 | | read --nodes 1, 20 sessions | 189,500 → 198,900 (+5 %) | 49.7 → 48.2 (−3 %) | 4,624 → 4,623 | | read --nodes 1, 200 sessions | 174,500 → 179,100 (+3 %) | 50.2 → 44.7 (**−11 %**) | 4,510 → 4,521 | | browse | 36,490 → 35,750 (−2 %) | 179 → 175 (−3 %) | 39,004 → 39,016 | | sub (5 × 100 items, 100 ms) | 94,020 → 93,180 notif/s | 8.2 → 9.2 (rounds 4.5–12.7) | 145 → 145 | **Where it helps:** CPU per request falls by 6 % on 4 cores, plain and encrypted, and by 3–11 % in the read scenarios with the .NET client. **Where it does not:** on 24 cores, CPU per request is within the spread of the rounds. There the cost is dominated by workers of one channel waiting for each other (the request hop and the send-order lock), which this PR does not change. ### Profile cpu trace, 24 cores, 10×100, quiet runs at 433–439k req/s. The trace was taken with this PR and #4788 (R7) together; the frames below belong to this PR only. | Frame | master | this PR | |---|---|---| | `FairRequestQueue.GetRevalidationStatus`, inclusive | 15.4 % | 14.0 % | | of which the provider and session lookups | 15.4 % | 3.9 % (`IsAdmissionCurrent`, mostly the session's expiry clock) | | `SessionManager.TryGetSessionContext`, inclusive | 14.6 % | 0.2 % (admission only) | The lookups are gone, but the dequeue-time check still accounts for about 10 % of the samples as its own time. That is the first touch of the request (entry, request object, channel context) on the worker thread, right after the receive thread wrote it on another core. On master the same cache misses showed up inside `TryGetSessionContext`, which is why the profile share (16.5 %) overstated what this change could save. Removing the request hop (separate work) is what removes those misses. ## Changes ### Evidence with the classification - **Internal interface.** `DefaultServerResourceIsolationProvider` implements a new internal interface, `IResourceIsolationAdmissionEvidence`. Its `Classify` also returns the evidence: the issued owner identity, which holds the channel snapshot and the session binding. - **Kept on the entry.** `FairRequestQueue` keeps the evidence, an internal `ResourceIsolationAdmissionEvidence` record that only the issuing provider interprets, on the queue entry. - **Confirmation.** Before the full revalidation, the queue asks the provider whether the evidence still holds. This applies to both checks, at dequeue and before execution. The provider: - compares the channel snapshot with the request's channel context, as before, so mutated channel evidence is still detected; - asks the binding's liveness check. Only a confirmed Good is short-circuited. - **No evidence, so every revalidation runs in full:** - a classifier is configured, because it may map the same inputs differently at any time; - the request has a token but no binding, because an activation may bind it while the request waits; - the binding comes from another binding provider, which has no liveness check. ### Binding liveness (`SessionManager`) A `SessionBindingContext` issued by `SessionManager` carries an internal liveness check. It runs the checks of `TryGetSessionContext` that can change while the binding and the request's channel context stay the same, without the token and session dictionary lookups: - **The dispatch binding is still this binding.** A removal, a new activation, an identity change, a transfer and an activation in commit all replace it. A commit always creates a new instance. - **The session's channel certificate was not replaced.** `SetRestoredSessionTransferSecurityState` can replace it without a new binding, so the certificate has a version. - **The manager is not shutting down.** Shutdown empties the token index without replacing the bindings. - **The session itself** is activated, not closing or claimed, not expired (the clock-based `HasExpired`, including expiry before the monitor closes the session), and still valid for the channel. These checks run in the same order as in `TryGetSessionContext`. A replaced binding releases its session, so owners that still reference the binding do not keep a closed session alive. ## Testing - **`RuntimeResourceIsolationTests` (new):** - the evidence confirms only an unchanged binding and channel, and a mutated certificate ends it without consulting the session; - it is withheld with a classifier, for a binding without liveness, and for a token without a binding; - a disposed provider throws; - the check allocates nothing. - **`SessionBindingTests` (new):** - the liveness check agrees with a fresh `TryGetSessionContext` through re-activation, transfer and close, an identity change, expiry before the monitor closes the session, an activation in commit, a session that starts closing, and shutdown; - replacing the transfer-security certificate with `SetRestoredSessionTransferSecurityState` keeps the binding but ends the liveness check (fails without the certificate version); - a race of 1000 transfers and a close against 4 readers never confirms a replaced binding, or any binding after the close. - **`FairRequestQueueTests` (new):** - current evidence replaces both full revalidations; - stale evidence at dequeue, or before execution, falls back to the full revalidation and its status (`BadSessionIdInvalid`, `BadServerTooBusy`), and the request does not run. - **Local, net10.0**, with both PRs applied, all 0 failed: - Opc.Ua.Core.Tests (Stack.Server, Stack.Tcp, Stack.Transport): 2753 passed; - Opc.Ua.Server.Tests: 8750 passed; - Opc.Ua.Sessions.Tests: 857 passed (379 skipped as on master); - Opc.Ua.Redundancy.Server.Tests: 884 passed; Opc.Ua.Redundancy.Client.Tests: 134 passed; - Opc.Ua.Stress.Tests Contract + Integration: 45 passed. net48 builds. `dotnet format whitespace --verify-no-changes` passes on the changed files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Marc Schier
|
d764161a53 |
Extract core/types fixes for schemas, replay, and source generation (#4801)
# Description Extract the reusable core/types layer from #4462 without including UaLens or the other planned extraction layers. - Preserve null versus empty in `ByteString.Copy`, and provide older-framework path, random-byte, and SHA-256 convenience APIs. - Encode abstract numeric structure fields (`Number`, `Integer`, and `UInteger`) as Variants in JSON, XSD, and BSD schemas, with regression coverage and schema documentation. - Add opt-in complete-file validation for finalized PCAP replay while retaining the existing tolerant reader default. - Keep placeholder references on generated type templates, not runtime instances, while preserving ordinary external references. **Initial extraction:** commit `8881e096fb5e4eaa36f3c13f8caa57733373bd19` preserves the assigned changes from source `390c4a7a51b71da40caec2d22ef74518bbe670ce`, based on pinned `master` commit `5b03e9afa492cdb36049ee52ae844f630f89d8db`: 20 exact full-file extractions and only the generic replay paragraph in `docs/Diagnostics.md`. The UaLens capture-window paragraph remains with the source PR. No UaLens projects, UI, native-rendering paths, dependencies, or workflows are included. **Review follow-up:** `814c0f2dc6056ea5eb898611565b3e96cfa65104` fixes direct-placeholder references that bypassed filtering through `TargetPath` and emitted an undefined `forInstance` guard in instance-only factories. Filtering now follows target resolution. Compilation/reference-shape regressions cover ObjectType/VariableType, both placeholder rules, and direct-child/external targets; `docs/NodeManagers.md` documents the behavior. The review thread was resolved after publication and verification. **CI follow-up:** `93ec3928bd5a9cb2ea86110085e79131db17d443` addresses both causes found in [CI run 38700770996](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/38700770996): - `MODELGEN012` treated an identical payload-less transitive dependency declaration as a competing model producer after the SDK Programs model appeared in newer `master`. Matching closure rows no longer emit that diagnostic. Producer ordering is unchanged; actual producers, differing declarations, and malformed payload handling retain their existing diagnostics. New regression cases exercise both reference orders, and `docs/ModelDependencies.md` records the distinction. - The queue-fairness test sampled OS scheduling and failed on macOS. This reuses the exact one-file correction from `84d2c34d3902bfe700a87f6c30708b435500878f` in #4765: hold the existing heavy-owner publication hook, prove concurrent work-conserving progress, then assert exact recovered 1:3 service, all 960 unique requests, and complete queue/token/owner/lease cleanup. No production scheduler or fairness bound was changed. The PR now changes 26 paths. Its pinned extraction ancestry is preserved: no rebase, force push, or merge of newer `master` was performed. This remains the bottom of native stack 4807, ordered 4801 -> 4802 -> 4803 -> 4804 -> 4806. ### Current CI-repair validation Windows, existing .NET SDK **10.0.401**, **Release**, with analyzers enabled. CI tested merge `9a2d252793da12aeb99ed379dfc9b03e89c912b7` (master `d40843c23a5fe8116ffe380cf937d88759f5218b` plus PR head `814c0f2dc6056ea5eb898611565b3e96cfa65104`), which includes newer tests absent from the pinned extraction branch. That exact merge was reproduced in a disposable detached worktree, then the proposed patch was applied. Git patch-id verified that the tested patch and committed branch patch were identical (`b7ed3ba2b61d1862825dd33d93c50c64d3a759ea`). Before the fix, all four failing client-accessor cases reproduced `MODELGEN012`, and both new matching-reexport cases failed. After the fix: | Context / scope | Framework | Passed | Failed | Skipped | | --- | --- | ---: | ---: | ---: | | Exact CI merge + patch: client-accessor and dependency fixtures | net10.0 | 32 | 0 | 0 | | Same fixtures with CI coverage instrumentation | net10.0 | 32 | 0 | 0 | | Exact CI merge + patch: entire SourceGeneration.Tests project | net10.0 | 295 | 0 | 0 | | Exact CI merge + patch: entire SourceGeneration.Tests project | net48 | 295 | 0 | 0 | | Preserved PR branch: dependency and placeholder fixtures | net10.0 | 30 | 0 | 0 | | Preserved PR branch: dependency and placeholder fixtures | net48 | 30 | 0 | 0 | | Preserved PR branch: entire FairRequestQueueTests fixture, with coverage | net10.0 | 93 | 0 | 0 | | Preserved PR branch: entire FairRequestQueueTests fixture | net48 | 93 | 0 | 0 | | Exact CI merge + patch: entire FairRequestQueueTests fixture, with coverage | net10.0 | 93 | 0 | 0 | All nine commands exited **0**. The repository TRX verdict helpers verified **993 passing test executions, 0 failures, and 0 fixture setup/teardown failures** across these overlapping runs. No build/analyzer warnings were reported. Normal restore resolved missing assets/target entries in the new checkout and when switching TFMs; no dependency declaration or SDK pin was changed. Exact successful test argument sets follow. Variables abbreviate the executable, project paths, and results directory only; output was also captured to separate local logs. ```powershell $env:DOTNET_ROOT = 'D:\git\UA-.NETStandard_master\TestResults\ualens-528-deep-repro\sdk' $env:PATH = "$env:DOTNET_ROOT;$env:PATH" $dotnet = "$env:DOTNET_ROOT\dotnet.exe" $generator = 'tests\Opc.Ua.SourceGeneration.Tests\Opc.Ua.SourceGeneration.Tests.csproj' $coreTests = 'tests\Opc.Ua.Core.Tests\Opc.Ua.Core.Tests.csproj' $results = 'C:\Users\mschier\.copilot\session-state\8f497e89-7fc2-4f48-880b-4803d97c1c12\files\ci-validation' # In the disposable exact-CI-merge checkout, with the identical proposed patch applied: Set-Location 'D:\Temp\ua-ci4801-8f497e89' # Passed: 32, Failed: 0, Skipped: 0 & $dotnet test $generator --no-restore --configuration Release --framework net10.0 -p:CustomTestTarget=net10.0 -p:RunAnalyzers=true -p:RunAnalyzersDuringBuild=true --filter 'FullyQualifiedName~ClientAccessorGeneratorTests|FullyQualifiedName~ModelDependencyScannerTests' --logger 'trx;LogFileName=ci-metadata-green-net10.trx' --results-directory $results --verbosity minimal # Passed: 295, Failed: 0, Skipped: 0; reuses the analyzer-enabled build above & $dotnet test $generator --no-build --no-restore --configuration Release --framework net10.0 -p:CustomTestTarget=net10.0 --logger 'trx;LogFileName=ci-sourcegeneration-merge-net10.trx' --results-directory $results --verbosity quiet # Passed: 295, Failed: 0, Skipped: 0 & $dotnet test $generator --configuration Release --framework net48 -p:CustomTestTarget=net48 -p:RunAnalyzers=true -p:RunAnalyzersDuringBuild=true --logger 'trx;LogFileName=ci-sourcegeneration-merge-net48.trx' --results-directory $results --verbosity minimal # Passed: 93, Failed: 0, Skipped: 0 & $dotnet test $coreTests --configuration Release --framework net10.0 -p:CustomTestTarget=net10.0 -p:RunAnalyzers=true -p:RunAnalyzersDuringBuild=true -p:CollectCoverage=true --collect 'XPlat Code Coverage' --settings 'tests\coverlet.runsettings.xml' --filter 'FullyQualifiedName~FairRequestQueueTests' --logger 'trx;LogFileName=ci-fairqueue-merge-net10.trx' --results-directory $results --verbosity minimal # Passed: 32, Failed: 0, Skipped: 0 & $dotnet test $generator --configuration Release --framework net10.0 -p:CustomTestTarget=net10.0 -p:RunAnalyzers=true -p:RunAnalyzersDuringBuild=true -p:CollectCoverage=true --collect 'XPlat Code Coverage' --settings 'tests\coverlet.runsettings.xml' --filter 'FullyQualifiedName~ClientAccessorGeneratorTests|FullyQualifiedName~ModelDependencyScannerTests' --logger 'trx;LogFileName=ci-metadata-coverage-net10.trx' --results-directory $results --verbosity minimal # In the preserved PR branch worktree: Set-Location 'D:\git\copilot-worktrees\UA-wot-d3-final\marcschier-congenial-umbrella' # Passed: 30, Failed: 0, Skipped: 0 & $dotnet test $generator --configuration Release --framework net10.0 -p:CustomTestTarget=net10.0 -p:RunAnalyzers=true -p:RunAnalyzersDuringBuild=true --filter 'FullyQualifiedName~ModelDependencyScannerTests|FullyQualifiedName~PlaceholderReferenceGeneratorTests' --logger 'trx;LogFileName=ci-sourcegeneration-branch-net10.trx' --results-directory $results --verbosity minimal # Passed: 93, Failed: 0, Skipped: 0 & $dotnet test $coreTests --no-restore --configuration Release --framework net10.0 -p:CustomTestTarget=net10.0 -p:RunAnalyzers=true -p:RunAnalyzersDuringBuild=true -p:CollectCoverage=true --collect 'XPlat Code Coverage' --settings 'tests\coverlet.runsettings.xml' --filter 'FullyQualifiedName~FairRequestQueueTests' --logger 'trx;LogFileName=ci-fairqueue-branch-net10.trx' --results-directory $results --verbosity minimal # Passed: 93, Failed: 0, Skipped: 0 & $dotnet test $coreTests --configuration Release --framework net48 -p:CustomTestTarget=net48 -p:RunAnalyzers=true -p:RunAnalyzersDuringBuild=true --filter 'FullyQualifiedName~FairRequestQueueTests' --logger 'trx;LogFileName=ci-fairqueue-branch-net48.trx' --results-directory $results --verbosity minimal # Passed: 30, Failed: 0, Skipped: 0 & $dotnet test $generator --configuration Release --framework net48 -p:CustomTestTarget=net48 -p:RunAnalyzers=true -p:RunAnalyzersDuringBuild=true --filter 'FullyQualifiedName~ModelDependencyScannerTests|FullyQualifiedName~PlaceholderReferenceGeneratorTests' --logger 'trx;LogFileName=ci-sourcegeneration-branch-net48.trx' --results-directory $results --verbosity minimal ``` `git diff --cached --check` passed. The queue-test blob was verified byte-for-byte against the existing published correction. The upstream branch was verified as `93ec3928bd5a9cb2ea86110085e79131db17d443` after push. **Remote CI is pending, not claimed green:** [new CI run 38712127138](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/38712127138). Cross-platform confirmation, CodeQL, coverage gates, and the complete `UA.slnx` suites on this new head are not established by these local checks. The source PR's checks are not claimed as evidence for this branch. ### Earlier validation The original extraction had 1,407 passing scoped test executions across net10.0/net48. The placeholder-review follow-up had six passing focused cases and 269 passing tests in the full source-generator project on each framework, with analyzer-enabled builds. Those are historical results for their respective commits, not fresh full-suite evidence for the CI-repair head. Earlier command details remain in this PR's edit history. ## Related Issues - Related to #4461 (existing tracking issue; not auto-closing). - Extracted from #4462 (source PR; remains open and is not auto-closing). - Reuses the bounded queue-test correction from #4765; does not incorporate that PR's other changes. _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ This began as an exact extraction of existing behavior. The subsequent commits address the reported placeholder-generation defect and observed CI failures; they are not a new design or a claim of design sign-off. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] 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. - [x] I have addressed **all** PR feedback received. |
||
|
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> |
||
|
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 |
||
|
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` (
|
||
|
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> |
||
|
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> |
||
|
|
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> |