-
Notifications
You must be signed in to change notification settings - Fork 512
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: Disable AggressiveAttributeTrimming with ILLink #18545
Conversation
</RuntimeHostConfigurationOption> | ||
|
||
<!-- Explicitly root the framework assembly due to NativeAOT trimming incompatibility tracked: https://github.com/dotnet/runtime/issues/86649 --> | ||
<TrimmerRootAssembly Include="Microsoft.iOS" /> |
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.
<TrimmerRootAssembly Include="Microsoft.iOS" /> | |
<TrimmerRootAssembly Include="$(_PlatformAssemblyName)" /> |
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.
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.
Thank you for the correction, it was an oversight. I corrected it.
I suppose with #18519 and #18529 this would no longer be necessary, correct?
If I am not mistaken, there is still an issue with ILC not knowing about [Preserve]
. The issue I noted in the comment should track/document all these incompatibilities.
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.
If I am not mistaken, there is still an issue with ILC not knowing about
[Preserve]
.
I am almost sure that [Preserve]
in the platform assembly is used only for the constructors that would get rooted by the generated code in #18519.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Test failures don't seem to be related. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The tests failures do not seem to be related. |
cc @dalexsoto @mandel-macaque - another one we are targeting for Preview 7 |
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
📚 [PR Build] Artifacts 📚Packages generatedView packagesPipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻✅ All tests on macOS M1 - Mac Big Sur (11.5) passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻✅ All tests on macOS M1 - Mac Ventura (13.0) passed. Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)
❗ API diff vs stable (Breaking changes).NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: simulator tests. 🎉 All 92 tests passed 🎉 Tests counts
Pipeline on Agent |
In the current setup with NativeAOT, during app build we run both ILLink and ILCompiler in cascade.
When
_AggressiveAttributeTrimming
feature switch is set totrue
ILLink removesIsTrimmable
attribute from assemblies, which means that when we are also running inTrimMode=partial
(which translates to--defaultrooting
ILC command-line arguments) ILC trimming is disabled completely.This PR disables
_AggressiveAttributeTrimming
in the first pass ie during trimming by ILLink and enables it in the second trimming pass performed by ILCompiler.Additionally, to workaround ILCompiler incompatibility with
Microsoft.iOS
(and friends) this platform assembly is explicitly rooted when passed to ILCompiler for trimming (this will be fixed once dotnet/runtime#86649 is resolved).Estimated savings: This change reduces the size of the application bundle by
0,58Mb
(or ~4,3% compared to the baseline)Fixes: #18479