Optimize calls to enum's GetHashCode/Equals #2117
Conversation
|
@davidwrighton PTLA |
davidwrighton
left a comment
There was a problem hiding this comment.
I like it, but we shouldn't put this kind of implementation specific api onto type system context. (In case we wish to attach the jit interface to a non-CompilerTypeSystemContext world.)
|
|
||
| private EnumInfoHashtable _enumInfoHashtable = new EnumInfoHashtable(); | ||
|
|
||
| public MethodDesc TryResolveConstrainedEnumMethod(TypeDesc enumType, MethodDesc virtualMethod) |
There was a problem hiding this comment.
This api shouldn't be part of the type system context that is specific to the compiler. We may wish to use it elsewhere. Please make it either an api surface exposed on the basic TypeSystemContext, or just make a separate non-typesystem joined api for this.
| codeStream.Emit(ILOpcode.ldflda, emitter.NewToken(Context.GetWellKnownType(WellKnownType.Object).GetKnownField("m_pEEType"))); | ||
| codeStream.EmitLdc(Context.Target.PointerSize); | ||
| codeStream.Emit(ILOpcode.add); | ||
| codeStream.Emit(loadIndirectOpcode); |
There was a problem hiding this comment.
Note to self: can be replaced with ldobj to get rid of the 10 line switch block...
* Adding ValueTuple * Adding ITuple * Go back to EqualityComparer.Default until optimization #2117 is implemented
|
I'm letting this sit because the code review feedback requires changes that overhaul a bunch of unrelated places (e.g. to make this not gross, delegate method injection needs to get the same treatment as this would get). I'm just waiting it out until the requirements for "jit interface to a non-CompilerTypeSystemContext world" stop being nebulous. This will get merged, eventually. |
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.
The common pattern is to go down to `UnderlyingType` whenever we don't care about enums instead of special casing them.
afdf66e to
3b53150
Compare
|
@davidwrighton Updated. Let's get this checked in! |
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 #2111.