Skip to content

Conversation

@stephentoub
Copy link
Member

Description

All comparisons are being performed with object.Equals(object). If T is a value type, every comparison ends up boxing the argument.

Customer Impact

Less boxing means less garbage generated and less time spent on collections.

Regression

No

Testing

Just CI

Risk

Minimal. EqualityComparer<T>.Default.Equals will use object.Equals if the type doesn't provide an IEquatable<T> implementation.

All comparisons are being performed with object.Equals(object).  If T is a value type, every comparison ends up boxing the argument.
@stephentoub stephentoub requested a review from a team as a code owner June 23, 2021 10:44
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jun 23, 2021
@ghost ghost requested review from SamBent, fabiant3 and ryalanms June 23, 2021 10:44
@lindexi
Copy link
Member

lindexi commented Jun 23, 2021

The other PR: #4220

@ThomasGoulet73
Copy link
Contributor

Would it be worth to do something like Dictionary to devirtualize when it's a value type and cache when it's a reference type ?
https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs#L274-L298

@stephentoub
Copy link
Member Author

Would it be worth to do something like Dictionary to devirtualize when it's a value type and cache when it's a reference type ?

I don't think so. We don't do so in the vast majority of use cases. The difference is tiny and only relevant in the hottest of hot paths when the rest of the overheads are tiny. It's also the kind of thing that is temporarily an optimization but could easily become a deoptimization as the JIT improves, e.g. it should become unnecessary in dictionary once dotnet/runtime#50446 is merged.

@ryalanms ryalanms merged commit 745c149 into dotnet:main Aug 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants