Use FrozenDictionary

[MikeAlhayek]

@sebastienros It would be nice to see if the signification performance gain when switching to Immutable collections when possible.

I am guessing we'll only see real difference if we have lots of parts registered

[dannyd89]

How is this a performance gain? ImmutableDictionary is slower than a normal Dictionary, for example see the discussion here. If you want faster Read times and we dont care about the time it takes to build the dictionary, why not use a FrozenDictionary?

Sorry for being curious

[MikeAlhayek]

@dannyd89 it was based on a convo I had with @sebastienros . Although I think he suggested to use FrozenDictionary. We'll probably need to do a pref testing with .net 8.

[sebastienros]

This PR is creating the collections/dictionaries once so this is fine. Better use FrozenDictionary however since they are more optimized for fast read-only operations (lookups).

[MikeAlhayek]

@dannyd89 FrozenDictionary are optimized for read and provide a better lookup performance. However, they are more expensive to create. The changes in the PR create the dictionary only once and make cache it. This means we pay the high cost for creating it once but improve the repeated lookup against.

But yes, we should not use ImmuableDictionary since it's much slower. Here is a benchmark I just did against different dictionary using 10k items with a string key and doing about 1000 lookups against the Dictionary, ImmutableDictionary and FrozenDictionary.

https://startdebugging.net/2023/08/net-8-performance-dictionary-vs-frozendictionary/

[dannyd89]

Yeah the benchmark really paints the picture, thanks!
ImmutableDictionary and in this regard anything starting with Immutable... are really niche. Most of the time it's just better to use Dictionary or now FrozenDictionary.

Are there currently some works to improve the performance of OC in general? Any tracking issues where I could also contribute?

[MikeAlhayek]

@dannyd89 there isn't really a list of performance based issues for OC. We like to merge PR #14572 as it is expected to bring notable improvement since serialization and deserialization is something we heavily use in OC and YesSql.

If you want to improve OC, the todo list is quite lengthy. you can filter by 1.x milestone on the issues and cherry pick which every issue you want to work on. We love any contributions that would improve OC https://github.com/OrchardCMS/OrchardCore/issues?q=is%3Aopen+is%3Aissue+milestone%3A1.x

[sebastienros]

NB: You see how the SortBuilders property is used during configuration and then revoked once the public properties are used (and transformed to FrozenDictionary). I think this is the same pattern that should be used for JsonSerialization types, instead of registering these in DI, just have the collection initialized in Options. And the DI will resolve an options object instead of a collection of services (cheaper).