Repository navigation
[Server] Implement smaller Performance improvements found when profiling - #3584
Merged
Merged
Conversation
Added GetNamespaceMetadataState(ushort) to ConfigurationNodeManager and interface for efficient lookup by namespace index. Switched to ConcurrentDictionary for thread safety. Updated AsyncCustomNodeManager to use index-based lookup. Cleared index cache in Clear method.
- Use WrappedValue assignment for DataValue in MonitoredNode and NodeState to align with updated Variant usage. - Return ServiceResult objects instead of raw StatusCodes for bad attribute IDs. - Update ServiceResult exception handling: include stack trace only in DEBUG, message only in release. - Optimize StatusCode to ServiceResult conversion by returning static Good/Bad instances.
Added SubscriptionTests to verify PublishTimerExpired behavior, including timer expiry, monitored item readiness, keep-alive, and triggering scenarios. Updated Subscription.cs only process triggering if triggered items are configured, improving correctness and efficiency.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces targeted performance optimizations across the server stack (publishing/monitored items, namespace metadata lookup, and ServiceResult/Variant handling) and adds tests/load tests to validate behavior under these changes.
Changes:
- Add namespace-index-based
NamespaceMetadataStatelookup and caching, and update node managers to use it. - Optimize publishing hot paths (MonitoredItem/Subscription), ServiceResult conversion, and Variant/DataValue handling.
- Add new unit tests for publishing readiness/triggering and new explicit client load tests (subscribe/read).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/Opc.Ua.Server.Tests/SubscriptionTests.cs | New tests covering Subscription.PublishTimerExpired behavior including triggering paths. |
| Tests/Opc.Ua.Server.Tests/MonitoredItemIsReadyToPublishTests.cs | New tests validating MonitoredItem.IsReadyToPublish across modes and aggregation. |
| Tests/Opc.Ua.Client.Tests/LoadTest.cs | Rename subscribe load test, adjust delay calculation, add explicit read load test. |
| Tests/Opc.Ua.Client.Tests/ClientTestFramework.cs | Adjust server fixture configuration for higher request concurrency. |
| Stack/Opc.Ua.Types/Utils/ServiceResult.cs | Release/Debug AdditionalInfo behavior; optimize StatusCode→ServiceResult conversion for Good/Bad. |
| Stack/Opc.Ua.Types/State/NodeState.cs | Use WrappedValue assignment and return ServiceResult for invalid attribute IDs. |
| Libraries/Opc.Ua.Server/Subscription/Subscription.cs | Micro-optimizations around publishing readiness and triggering checks. |
| Libraries/Opc.Ua.Server/Subscription/MonitoredItem/MonitoredItem.cs | Reduce locking on hot properties; adjust tracing/logging overhead. |
| Libraries/Opc.Ua.Server/NodeManager/MonitoredItem/MonitoredNode.cs | Use WrappedValue initialization on queued DataValues. |
| Libraries/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs | Switch to namespace-index-based metadata lookup. |
| Libraries/Opc.Ua.Server/Configuration/IConfigurationNodeManager.cs | Add GetNamespaceMetadataState(ushort namespaceIndex) to the interface. |
| Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs | Implement index-based lookup; switch caches to ConcurrentDictionary and clear index cache on namespace changes. |
marcschier
approved these changes
Feb 27, 2026
| if (!m_readyToPublish) | ||
| { | ||
| ServerUtils.EventLog.MonitoredItemReady(Id, "FALSE"); | ||
| //ServerUtils.EventLog.MonitoredItemReady(Id, "FALSE"); |
Collaborator
There was a problem hiding this comment.
Id say Lets just clean this up and remove it all, including the EventLog part in ServerUtils.
This was referenced Oct 8, 2026
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.
Proposed changes
-Added GetNamespaceMetadataState(ushort) to ConfigurationNodeManager and interface for efficient lookup by namespace index. Switched to ConcurrentDictionary for thread safety. Updated AsyncCustomNodeManager to use index-based lookup. Cleared index cache in Clear method.
Types of changes
Checklist
Further comments