Skip to content

Pre-size intermediate Dictionary in ToFrozenDictionary#128300

Open
AndrewP-GH wants to merge 5 commits into
dotnet:mainfrom
AndrewP-GH:perf/frozen-dict-presize
Open

Pre-size intermediate Dictionary in ToFrozenDictionary#128300
AndrewP-GH wants to merge 5 commits into
dotnet:mainfrom
AndrewP-GH:perf/frozen-dict-presize

Conversation

@AndrewP-GH
Copy link
Copy Markdown

@AndrewP-GH AndrewP-GH commented May 17, 2026

ToFrozenDictionary builds an intermediate Dictionary<,> to deduplicate keys when the source isn't already a matching Dictionary<,>. It used the default-capacity constructor, so a known-size source pays log2(N) resizes and ~2N rehashes for nothing.

Helps most where the early source as Dictionary<,> reuse path doesn't fire: ConcurrentDictionary<,>, SortedDictionary<,>, ImmutableDictionary<,>, ReadOnlyDictionary<,>, plus arrays and List<KVP> from LINQ.

Sizing-only change — Dictionary<,> capacity is contractually behavior-invariant, so existing MultipleValuesSameKey_LastInSourceWins already covers the shared foreach { d[k] = v; } body.

Copilot AI review requested due to automatic review settings May 17, 2026 14:28
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label May 17, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Improves ToFrozenDictionary construction performance by pre-sizing the intermediate Dictionary when the source reports a count, and adds a regression test to ensure duplicate-key “last in wins” semantics remain intact for ICollection sources.

Changes:

  • Pre-size the intermediate Dictionary<TKey, TValue> using the source collection’s count when available.
  • Add a unit test covering duplicate keys for ICollection sources to prevent regressions in overwrite semantics.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs Pre-sizes intermediate dictionary based on source count to reduce resizes/rehashes during population.
src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs Adds regression coverage to ensure pre-sizing doesn’t change duplicate-key overwrite behavior.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

@AndrewP-GH AndrewP-GH force-pushed the perf/frozen-dict-presize branch 2 times, most recently from 1f15afe to 1ce0801 Compare May 17, 2026 14:35
Copilot AI review requested due to automatic review settings May 17, 2026 14:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs Outdated
Comment thread src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs Outdated
When the source is not already a Dictionary, ToFrozenDictionary builds
an intermediate Dictionary to deduplicate keys with last-wins semantics.
The previous code used the default-capacity constructor, which paid
log2(N) resizes and ~2N redundant rehashes when populating from a known
size source such as an array or List.

Use ICollection<KeyValuePair>.Count as a capacity hint when available,
mirroring the HashSet(IEnumerable, IEqualityComparer) constructor.
The ReadOnlySpan overload of FrozenDictionary.Create already pre-sizes,
so this brings the IEnumerable path in line with it.
@AndrewP-GH AndrewP-GH force-pushed the perf/frozen-dict-presize branch from 1ce0801 to cbeb324 Compare May 17, 2026 14:37
Switch the source-type check from ICollection<KeyValuePair> to
IDictionary<,> / IReadOnlyDictionary<,>. For dictionary inputs Count
reflects already-unique keys, so the intermediate Dictionary is sized
exactly without risk of over-allocation from duplicate-bearing
collections such as KeyValuePair[].
Copilot AI review requested due to automatic review settings May 17, 2026 14:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@AndrewP-GH
Copy link
Copy Markdown
Author

@dotnet-policy-service agree company="Dodo Brands"

Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Copilot AI review requested due to automatic review settings May 17, 2026 14:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

AndrewP-GH and others added 2 commits May 17, 2026 18:15
IDictionary<TKey, TValue> extends ICollection<KeyValuePair<TKey, TValue>>
and IReadOnlyDictionary<TKey, TValue> extends IReadOnlyCollection<KeyValuePair<TKey, TValue>>,
and Count is inherited from the collection interfaces. The dictionary
arms always matched the collection arms with the same value, so they
were dead code. Matches the pattern Dictionary<,> and ConcurrentDictionary<,>
use in their own IEnumerable constructors.
Copilot AI review requested due to automatic review settings May 17, 2026 15:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Collections community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants