-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[NativeAOT] Properly implement System.Linq.Expressions
substitutions according to the recent refactoring
#89638
[NativeAOT] Properly implement System.Linq.Expressions
substitutions according to the recent refactoring
#89638
Conversation
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue Details#89308 introduced a regression for NativeAOT because the changes assumed that ILC trimming can propagate constants over methods. As this is not true, the This PR fixes this regression, by adjusting the Additionally, I cleaned up a few places that had left overs of the removed private feature switches. Thanks @vitek-karas for figuring out the right approach. @dotnet/ilc-contrib Fixes #89527
|
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
@eerhardt I have measured the impact of this change with https://github.com/aspnet/Benchmarks/tree/main/src/BenchmarksApps/TodosApi on <ItemGroup>
<FrameworkReference Update="Microsoft.NETCore.App" RuntimeFrameworkVersion="8.0.0-dev" />
<PackageReference Include="Microsoft.DotNet.ILCompiler" Version="8.0.0-dev" />
</ItemGroup> The measurements show the following results:
These numbers do not match what you posted in: #89527 (comment) but I only have a MacBookPro M1 at hand, so am not able to verify the impact of this change on I also double-checked the mstat files. confirming the fix: |
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.
Neat!
#89308 introduced a regression for NativeAOT because the changes assumed that ILC trimming can propagate constants over methods. As this is not true, the
Ref.Emit
backend is not trimmed away by ILC anymore increasing the app size.This PR fixes this regression, by adjusting the
System.Linq.Expressions
substitutions to wireDynamicCodeSupported
feature switch value toget_CanCompileToIL
andget_CanCreateArbitraryDelegates
and substitute them withfalse
- avoiding the constant propagation over methods limitation.Additionally, I cleaned up a few places that had left overs of the removed private feature switches.
Verified that the regression has been fixed by adding a reference to
System.Linq.Expressions
in the HelloiOS sample and AOTing with and without changes in the PR:Thanks @vitek-karas for figuring out the right approach.
@dotnet/ilc-contrib
Fixes #89527