This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Avoid boxing in calls to enum's GetHashCode #7895
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jkotas
reviewed
Oct 31, 2016
src/vm/jitinterface.cpp
Outdated
| // We can't do this for any other method since ToString and Equals have different semantics for enums | ||
| // and their underlying type. | ||
| LPCUTF8 pMethodName = pMD->GetName(); | ||
| if (strcmp(pMethodName, "GetHashCode") == 0) |
Member
There was a problem hiding this comment.
pMD->GetSlot() == MscorlibBinder::GetMethod(METHOD__OBJECT__GET_HASH_CODE)->GetSlot() would be a bit more efficient.
MichalStrehovsky
added a commit
to MichalStrehovsky/corert
that referenced
this pull request
Nov 1, 2016
Generate synthetic instance methods into which constrained calls can resolve. This prevents boxing 'this' that would be required before a call to the System.Enum's implementation. This is specifically not a synthetic virtual override. The thunk only has very little perf advantage over System.Enum's implementation if 'this' is already boxed and is not worth the size overhead. This is an extended port of dotnet/coreclr#7895
MichalStrehovsky
added a commit
to MichalStrehovsky/corert
that referenced
this pull request
Mar 17, 2017
Generate synthetic instance methods into which constrained calls can resolve. This prevents boxing 'this' that would be required before a call to the System.Enum's implementation. This is specifically not a synthetic virtual override. The thunk only has very little perf advantage over System.Enum's implementation if 'this' is already boxed and is not worth the size overhead. This is an extended port of dotnet/coreclr#7895 Fixes dotnet#2111.
This was referenced Jan 31, 2020
Forgind
pushed a commit
to dotnet/msbuild
that referenced
this pull request
Aug 14, 2020
Calling GetHashCode() on an enum results in boxing (i.e. a GC allocation) when running on .NET Framework. This has been addressed in .NET Core (dotnet/coreclr#7895) but we want to fix it on our side still because it's showing in Visual Studio profiles: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1166715 This PR is also removing the calls to base.GetHashCode() which are pointless in structures that calculate the hashcode out of all their fields.
sharwell
added a commit
to sharwell/roslyn
that referenced
this pull request
Mar 29, 2021
Boxing was eliminated for calls to GetHashCode() on enum types in dotnet/coreclr#7895. This commit updates SegmentedDictionary<TKey, TValue> to avoid assuming this path is optimized for netstandard2.0 builds, since they might run on .NET Framework.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A real world app prompted me to look into this. I know this is not a huge deal because
EqualityComparer<T>.Defaulthas special casing for enums, but it's actually pretty easy to hit this anyway (even System.Private.CoreLib has a couple hits) and people on the internet are always puzzled about this one.The code generated for the above has an unexpected boxing operation in it. I say unexpected, because e.g.
1.GetHashCode()wouldn't have boxing.The reason why we end up with boxing is because in the constrained call to
System.Object::GetHashCode, the constraint resolution ends up resolving the method toSystem.Enum::GetHashCode()andSystem.Enumis a reference type. Bodies for instance methods on reference types expect a boxed this, while bodies on value types expect a byref.I'm patching it up to pretend the constrained call was to the underlying type's
GetHashCodemethod. I don't pretend it's a great solution and unfortunately it won't coverEquals. I'll understand if there's pushback against merging this.Before the change
After the change