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

Avoid boxing when structs are used as collection keys. #69443

Conversation

ToddGrun
Copy link
Contributor

@ToddGrun ToddGrun commented Aug 9, 2023

Change several structs to record structs such that they no longer are boxed when used as keys in MultiDictionary's internal ImmutableHashSet. When MultiDictionary's value is a struct and is created without specifying the value's IEqualityComparer, the default struct comparer is used. This equality comparer is notoriously slow due to boxing. By switching these structs to record structs, an implementation of IEquatable is generated and the default struct equality comparer isn't used.

Addresses #69165

Sam mentioned that there is an issue already logged in the roslyn-analyzers repo around adding an analyzer to detect this.

dotnet/roslyn-analyzers#1640

Change several structs to record structs such that they no longer are boxed when used as keys in MultiDictionary's internal ImmutableHashSet. When MultiDictionary's value is a struct and is created without specifying the value's IEqualityComparer, the default struct comparer is used. This equality comparer is notoriously slow due to boxing. By switching these structs to record structs, an implementation of IEquatable is generated and the default struct equality comparer isn't used.

Addresses dotnet#69165

Sam mentioned that there is an issue already logged in the roslyn-analyzers repo around adding an analyzer to detect this.

dotnet/roslyn-analyzers#1640
@ToddGrun ToddGrun requested a review from sharwell August 9, 2023 18:12
@ToddGrun ToddGrun requested a review from a team as a code owner August 9, 2023 18:12
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 9, 2023
@ToddGrun ToddGrun linked an issue Aug 9, 2023 that may be closed by this pull request
@@ -20,7 +20,7 @@ internal enum HighlightSpanKind
}

[DataContract]
internal readonly struct HighlightSpan
internal readonly record struct HighlightSpan
Copy link
Member

Choose a reason for hiding this comment

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

❓ Why is this one changing?

Copy link
Member

Choose a reason for hiding this comment

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

➡️ This type isn't used as a dictionary key, but rather as a dictionary value in a type that happens to create a set from the values.

@@ -63,11 +62,11 @@ private string GetDebuggerDisplay()
=> Name + ", " + ParentIndex;
}

private readonly struct ParameterTypeInfo(string name, bool isComplex, bool isArray)
private readonly record struct ParameterTypeInfo(string name, bool isComplex, bool isArray)
Copy link
Member

Choose a reason for hiding this comment

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

❓ Why is this one changing?

Copy link
Member

Choose a reason for hiding this comment

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

➡️ This type isn't used as a dictionary key, but rather as a dictionary value in a type that happens to create a set from the values.

@ToddGrun ToddGrun merged commit e6736ca into dotnet:main Aug 10, 2023
24 checks passed
@ghost ghost added this to the Next milestone Aug 10, 2023
@dibarbet dibarbet modified the milestones: Next, 17.8 P2 Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce boxing allocations of ExtensionMethodInfo in IDE side
5 participants