-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[MONO] Partially revert "Add TypeName APIs to simplify metadata lookup" #118761
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
Mono specific changes in dotnet#111598 regressed Mono runtime performance significantly. Reverting the changes for now. Fixes dotnet#112763
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 partially reverts Mono-specific changes from a previous TypeName APIs optimization that caused significant performance regressions in the Mono runtime. The revert focuses on removing the extensibleParser
parameter and related logic while preserving the core TypeName API functionality.
Key changes:
- Remove
extensibleParser
parameter from TypeNameResolver methods - Restore direct RuntimeType.GetType calls for simple overloads
- Revert nested type resolution to use standard GetNestedType method
- Update test expectations for Mono runtime differences
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
Type.Mono.cs | Reverts GetType overloads to call RuntimeType.GetType directly for simple cases, removes extensibleParser parameter |
RuntimeType.Mono.cs | Restores GetNestedType method signature and removes internal ignoreAmbiguousMatch logic |
TypeNameResolver.Mono.cs | Removes extensibleParser field and simplifies nested type resolution logic |
TypeTests.cs | Updates test data to account for Mono runtime behavior differences |
src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs
Show resolved
Hide resolved
src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs
Outdated
Show resolved
Hide resolved
Tagging subscribers to this area: @dotnet/area-system-reflection |
…time into typename-parsing-perf
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.
I build this branch locally (Release mode), then copied System.Private.CoreLib.dll
on top of my Android runtime pack.
This seems to fix this specific performance regression on Android:
before fix: 1023.8ms
after fix: 907ms
This is pretty close to my estimate from #118747, so looks good. 👍
I tested this with a "franken-build", using `System.Private.CoreLib.dll` from this PR: * dotnet/runtime#118761 With the `Type.GetType()` revert in place for Mono, I'm getting good startup on an Android Pixel 5 (old) device with the `dotnet new maui` project template: Comparing the updated AOT profile: Before: Average(ms): 624.9 Std Err(ms): 3.77844765302719 Std Dev(ms): 11.9485006032835 After: Average(ms): 582.1 Std Err(ms): 2.92289810519172 Std Dev(ms): 9.24301538099625 Some of my past numbers were on a Pixel 7, so these are good numbers for an even older device. We may want to run this again with newer (less-franken) builds, but this is worth merging for now.
I tested this with a "franken-build", using `System.Private.CoreLib.dll` from this PR: * dotnet/runtime#118761 With the `Type.GetType()` revert in place for Mono, I'm getting good startup on an Android Pixel 5 (old) device with the `dotnet new maui` project template: Comparing the updated AOT profile: Before: Average(ms): 624.9 Std Err(ms): 3.77844765302719 Std Dev(ms): 11.9485006032835 After: Average(ms): 582.1 Std Err(ms): 2.92289810519172 Std Dev(ms): 9.24301538099625 Some of my past numbers were on a Pixel 7, so these are good numbers for an even older device. We may want to run this again with newer (less-franken) builds, but this is worth merging for now.
I tested this with a "franken-build", using `System.Private.CoreLib.dll` from this PR: * dotnet/runtime#118761 With the `Type.GetType()` revert in place for Mono, I'm getting good startup on an Android Pixel 5 (old) device with the `dotnet new maui` project template: Comparing the updated AOT profile: Before: Average(ms): 624.9 Std Err(ms): 3.77844765302719 Std Dev(ms): 11.9485006032835 After: Average(ms): 582.1 Std Err(ms): 2.92289810519172 Std Dev(ms): 9.24301538099625 Some of my past numbers were on a Pixel 7, so these are good numbers for an even older device. We may want to run this again with newer (less-franken) builds, but this is worth merging for now.
Mono specific changes in #111598 regressed Mono runtime performance significantly. Reverting the changes for now.
Fixes #112763