-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Do not generate __GetFieldHelper for ValueTuple #117238
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
Conversation
Something I noticed in a log. Wonder if it's meaningful.
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
|
0.2% on a minimal Kestrel app. Maybe worth the two lines. Not likely to cause trouble in the future. Size statisticsPull request #117238
|
There was a problem hiding this 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 updates the value-type helper logic to skip generating __GetFieldHelper for System.ValueTuple types.
- Adds a special-case check to bypass helper generation for any
ValueTuplein the System module. - Returns
falseearly to avoid the default bit-comparison infrastructure.
Comments suppressed due to low confidence (1)
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.ValueTypeMethods.cs:106
- Add a unit test that verifies
ValueTupletypes bypass the__GetFieldHelperpath to ensure this special case isn’t accidentally removed in future refactorings.
if (valueType.Module == SystemModule && valueType.Name.StartsWith("ValueTuple`", StringComparison.OrdinalIgnoreCase) && valueType.Namespace == "System")
...coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.ValueTypeMethods.cs
Outdated
Show resolved
Hide resolved
...coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.ValueTypeMethods.cs
Outdated
Show resolved
Hide resolved
…eSystemContext.ValueTypeMethods.cs Co-authored-by: Copilot <[email protected]>
|
I see a few more recently added types missing in the lists for explicit skip. Double and Single are specialized but Half isn't. There are probably more, but not sure if you gain as much as for generic types. |
|
Double and Single are there because that's the legacy logic that has been in use for ValueType.Equals/GetHashCode since forever. It was never updated for the new floating point types (not on AOT side, not on JIT side). We recommend everyone to steer clear of the base ValueType.Equals/GetHashCode and provide their own override. |
Noticed a couple of these in a log.
Cc @dotnet/ilc-contrib