[msbuild] Add ILStrip'ing for net6 applications #12558
Conversation
- Controled by EnableAssemblyILStripping which defaults to true - Integration test included Before - https://gist.github.com/chamons/c7886f7bacbc2e5ac5966e4251d13e71 After - https://gist.github.com/chamons/148e1bef22fa336f953f3d02dcf20667 859,136 -> 527,872 managed
|
I'm marking this as do-not-merge as I have not yet run device/sim tests locally. I had to nuke the world, so it's going to be a number of hours before they complete. I wanted to get this reviewed in Rolf's TZ. |
| <UseAppHost>false</UseAppHost> | ||
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <EnableAssemblyILStripping Condition="'$(EnableAssemblyILStripping)' == ''">true</EnableAssemblyILStripping> |
There was a problem hiding this comment.
JIT (simulator) and interpreter (both sim and devices) needs IL to be present
In legacy the IL stripping is only done for
- device builds and
- release builds and
- AOT'ed assemblies (apps can mix and match AOT and interpreted assemblies)
There was a problem hiding this comment.
wrt release builds I'm not 100% sure the debugger needs IL
However stripping takes extra time and does not help the debugging (on device) experience/flow
| <UseAppHost>false</UseAppHost> | ||
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <EnableAssemblyILStripping Condition="'$(EnableAssemblyILStripping)' == ''">true</EnableAssemblyILStripping> |
There was a problem hiding this comment.
-
We only want this enabled by default when:
- Building with FullAOT, so:
- Not on macOS
- Not in the simulator
- Not when the interpreter is enabled
- Not on Mac Catalyst unless building for ARM64 and the interpreter is disabled (we can't JIT on Mac Catalyst/ARM64).
- Building for Release.
- Building with FullAOT, so:
-
We have a
_ParseBundlerArgumentstask that parses the extra mtouch/mmp arguments and writes the results to MSBuild properties:
https://github.com/xamarin/xamarin-macios/blob/f0373ee4f7140a70138fc919baf3fc3381656ef7/msbuild/Xamarin.Shared/Xamarin.Shared.targets#L1048-L1070
That task should be augmented to handle the --nostrip mtouch argument, and set this property accordingly.
Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
|
Woops, did not mean to nuke the branch. My mistake. |
|
Now in #12563 Apologies. |
Before - https://gist.github.com/chamons/c7886f7bacbc2e5ac5966e4251d13e71
After - https://gist.github.com/chamons/148e1bef22fa336f953f3d02dcf20667
859,136 -> 527,872 managed