Skip to content
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

Add FrozenDictionary.Create #114192

Merged
merged 1 commit into from
Apr 5, 2025

Conversation

stephentoub
Copy link
Member

Contributes to #114090

Adds FrozenDictionary.Create for creating a FrozenDictionary from a span of key/value pairs. This can be used directly and enables creating FrozenDictionary instances using C# 14 dictionary expressions.

Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

1 similar comment
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Copy link
Contributor

@Copilot 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

This PR adds two new overloaded methods FrozenDictionary.Create that accept a ReadOnlySpan of key/value pairs, enabling the use of C# 14 dictionary expressions and offering improved usability when constructing FrozenDictionary instances.

  • Added tests for both empty span and non-empty sources.
  • Updated the LookupItems and MultipleValuesSameKey tests to support both the legacy ToFrozenDictionary method and the new Create overloads.
  • Updated API reference and implementation to support the new FrozenDictionary.Create methods.

Reviewed Changes

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

File Description
tests/Frozen/FrozenDictionaryTests.cs Added test cases for empty span input and updated tests to conditionally use the new Create overload.
src/System/Collections/Frozen/FrozenDictionary.cs Introduced two new Create overloads using ReadOnlySpan for constructing FrozenDictionary instances.
ref/System.Collections.Immutable.cs Updated the public API reference to include the new FrozenDictionary.Create overloads.
Files not reviewed (1)
  • src/libraries/System.Collections.Immutable/src/CompatibilitySuppressions.xml: Language not supported

Copy link
Contributor

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

new EmptyFrozenDictionary<TKey, TValue>(comparer);
}

Dictionary<TKey, TValue> d = new(source.Length, comparer);
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity (out of scope for this PR), would it be possible to optimize the construction of frozen dictionaries from constant collections (as in, a collection expressions where all keys are compile time constants) so to avoid building the intermedia dictionary, and perhaps just enable some additional fast-path during construction? Would something like that be more like something that eg. ILC could do on NAOT, or something that Roslyn/.NET could improve in general?

I'm asking because we have a scenario in CsWinRT where we'd end up with a massive lookup table of string -> string that would have 700+ entries, and we're thinking about potential codegen strategies for it. An option would be a frozen dictionary, but paying for building all that at startup even though all values would just be constants feels fairly expensive 🤔

Copy link
Member

Choose a reason for hiding this comment

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

a massive lookup table of string -> string that would have 700+ entries

Internals of dotnet/runtime repo also come across similar requirements in areas like formats, globalization and security, we ended up source generating blittable RVA data fields with chain of switch-cases (and in some places without switch-cases using multiple ROS). An example of such generator which packs the lookup table in data section: sharplab (fill seedDataDictionary with your data). Would be nice to have something like this OOTB statically built data solely for lookup purposes (not necessarily via FrozenDictionary interface).

@stephentoub stephentoub merged commit c08ef4b into dotnet:main Apr 5, 2025
87 of 90 checks passed
@stephentoub stephentoub deleted the frozendictionarycreate branch April 5, 2025 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants