-
Notifications
You must be signed in to change notification settings - Fork 1k
Immutable Encodeable factory and lookup performance improvements #3241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces immutable encodeable factory with lookup performance improvements. The changes replace the existing mutable EncodeableFactory implementation with an immutable design that uses FrozenDictionary for optimized lookups and thread safety.
Key changes:
- Refactored EncodeableFactory to be immutable with a builder pattern for modifications
- Replaced ConcurrentDictionary with FrozenDictionary for 15-20% performance improvements in lookups
- Updated interfaces to separate factory creation from type lookup responsibilities
Reviewed Changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Stack/Opc.Ua.Core/Types/Encoders/EncodeableFactory.cs | Complete rewrite to immutable factory with FrozenDictionary and builder pattern |
| Stack/Opc.Ua.Core/Types/Encoders/IEncodeableFactory.cs | New interface design separating factory, builder, and lookup responsibilities |
| Stack/Opc.Ua.Core/Types/Utils/TypeInfo.cs | Added XML name utility methods moved from EncodeableFactory |
| Tests/Opc.Ua.Core.Tests/Types/Encoders/EncodeableFactoryTests.cs | Comprehensive test suite for new factory implementation |
| Multiple other files | Updates to use new factory interfaces and remove telemetry parameters |
Comments suppressed due to low confidence (1)
Stack/Opc.Ua.Core/Types/Utils/Utils.cs:1
- Unused imports should be removed. The removed using statements for System.Diagnostics.CodeAnalysis and Microsoft.Extensions.Logging are no longer needed.
/* Copyright (c) 1996-2022 The OPC Foundation. All rights reserved.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3241 +/- ##
==========================================
+ Coverage 57.50% 57.53% +0.03%
==========================================
Files 361 362 +1
Lines 79163 79215 +52
Branches 13821 13835 +14
==========================================
+ Hits 45521 45575 +54
- Misses 29404 29411 +7
+ Partials 4238 4229 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Proposed changes
This PR introduces immutable encodeable factory with lookup performance improvements. The changes replace the existing mutable EncodeableFactory implementation with an immutable design that uses FrozenDictionary for optimized lookups and thread safety.
Key changes:
Related Issues
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that apply. You can also fill these out after creating the PR.Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...