Replaced ICollection<string> with HashSet<string> for various properties in the OpenIdConnectConfiguration #3283
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.
Replaced
ICollection<string>withHashSet<string>for various properties in theOpenIdConnectConfigurationclassReplaced
ICollection<string>withHashSet<string>for various properties in theOpenIdConnectConfigurationclass. This change improves performance by utilizingHashSetfor faster lookups and uniqueness enforcement. Updated initialization inInterlocked.CompareExchangeto ensure efficient collection types are used.Description
OpenIdConnectConfigurationinternally usesSystem.Collections.ObjectModel.Collection<string>exposed asSystem.Collections.Generic.ICollection<T>.However, using
System.Collections.ObjectModel.Collection<string>instead ofSystem.Collections.Generic.List<T>has a few drawbacks, namely,Collection<T>introduces a slight overhead due to its virtual methods (InsertItem, RemoveItem, etc.), which can impact performance in high-throughput scenarios whileList<T>is optimized for speed and memory usage, making it better for performance-critical applications.However, I came across WSO2AM Identity Server OIDC discovery endpoint where I found these:
{ // (...) "Response_modes_supported": [ "query", "fragment", "form_post" ], // (...) "response_modes_supported": [ "query", "fragment", "form_post" ], // (...) }That results in this deserialization:
Using
HashSet<string>would solve this, and add:HashSet<string>guarantees no duplicates. If you try to add a duplicate, it simply won’t be added.List<string>, you’d need to manually check for duplicates usingContains, which is less efficient.HashSet<string>uses a hash-based structure, soContains,Add, andRemoveoperations are O(1) on average.List<string>uses a linear search, making these operations O(n).HashSet<string>, you don’t need to write extra logic to prevent duplicates or optimize lookups.It would be better to expose it as
Set<string>, but that would be a breaking change.As for the backing fields for the properties, there's no benefit in erasing their types. Let's keep the real type.