Use FrozenDictionary where appropriate

[JamesNK]

Is there an existing issue for this?

• I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

.NET 8 adds FrozenDictionary. It's a read-only dictionary with a higher creation cost but better read performance. Good for lookup dictionaries that live a long time.

See dotnet/runtime#67209

Describe the solution you'd like

Look to see where dictionaries are used today. Consider replacing them with FrozenDictionary.

Additional context

No response

[richiebijoy]

If you are looking for help on this.. I would be glad to take it up

It would be my first open-source contribution

Thanks In Advance :)

[danmoseley]

@richiebijoy you'd be welcome to.

@BrennanConroy @JamesNK do you have a suggestion for a dictionary that would be a good candidate? (And presumably one that could have perf measured, if needed)

[BrennanConroy]

Not too sure. We should do a pass of all Dictionary usage in the repo and look for cases where we construct it once and don't readd to it.

One example I found was

aspnetcore/src/Http/Routing/src/Matching/DictionaryJumpTable.cs

Line 13 in ff2241b

private readonly Dictionary<string, int> _dictionary;

where we construct the dictionary once, and I think it's only at startup which is ideal for the high cost of creating a frozen dictionary. But it should be double checked.

[richiebijoy]

Thank You so much for this opportunity @danmoseley

I'll look at this example to start with . Thanks for your help @BrennanConroy

:)

[adityamandaleeka]

Welcome to the repo @richiebijoy! Since you mentioned it'll be your first OSS contribution, if you need any support getting your first pull request going, let us know and one of us can help you.

[richiebijoy]

Thank you so much for the warm welcome, @adityamandaleeka ! I really appreciate your offer to help me with my first pull request. It's great to see such a supportive and welcoming community here. I'll definitely reach out if I need any assistance.

I'm still working on it. Took me a while to set up and I ran into some issues with my C Drive during that time, But everything is fine now

Thanks 😄

[richiebijoy]

Hi everyone. I have a quick question...
Are the existing tests enough or do I need to create new ones?

[richiebijoy]

I made the change in that one file and ran the existing tests. All passed.

I am guessing I should make each of these changes in separate PRs so that it is easier for the reviewer.
@adityamandaleeka Could you please help me with this. Thanks In Advance :)

[ilmax]

Given how expensive is to create a frozen dictionary, the change may end up regressing startup time. @richiebijoy did you run some benchmarks on current state vs frozen dictionary?

[danmoseley]

I should make each of these changes in separate PRs

I'd suggest having only one change in the first PR.

Just to warn you I expect that most of the work here is going to be performance validation. 🙂 You can do quick and dirty measurements first with Stopwatch to get an idea. Someone more familiar will have to suggest how to test startup properly and what scenario will realistically test it.

Are we assuming that after construction the lookups will be faster, or should we validate somehow? Obviously that's the assumption but FrozenDictionary is quite new and I don't think we have a sense of any performance gotchas yet. That would be good to understand before subsequent PR's.

[richiebijoy]

@danmoseley Okay I'll do that and in the mean time I'll also look up on how to test start up properly.
If you know anyone that can guide me in the right direction that would be a huge help.
Thanks a lot 😃