Fix how System.Linq.Expressions.dll is supported on iOS-like platforms
#87924
Comments
|
Tagging subscribers to 'os-ios': @steveisok, @akoeplinger, @kotlarmilos Issue DetailsDescriptionThere is a difference in how
ExplanationNativeAOTThe issue was called out by @MichalStrehovsky noting that codepaths in
are not AOT friendly (reported here). For desktop platforms, NativeAOT fixes this by:
When it comes to iOS-like platforms, above is not true. When
MonoAOTAs for MonoAOT is concerned, there were several issues reported against the
includes generating code for a lot of generic delegates and Mono tries its best to support these, but at what cost? It is true that choosing delegates to implement fast invocation of methods should have better performance, but in case with Mono and delegates over value types, the compiler will generate Risk assessmentPros
Cons
ProposalHere is the list of tasks which should resolve all the reported issues around
Thanks to @MichalStrehovsky @vargaz @rolfbjarne for helping to identify these issues.
|
|
/cc: @SamMonoRT @eerhardt |
|
Thanks Ivan. Can you or Michal explain the constant propagation issue? Is this some kind of IL trimmer bug - we substitute constants for feature switches all the time. Why is it a problem here? My understanding is that the existing delegate-based approach will work with Mono AOT now. |
I was asking about this multiple times already, and the answer was always that's it's intentionally not planned. .NET Native used to implement it (with different codegen backend), but any remains of the code were removed over time. I don't think that Linq.Expressions would have any effect on the decision as the library itself is in archived state. |
The problem is that two of the three mentioned control variables are configured as constants:
From my understanding, when the constant propagation is enabled during Linq.Expressions assembly for iOS-like platforms (this can be seen in the warning messages I pasted in the description).
Replacing them with a non-constant |
Ah, I see. It's not a trimmer bug. |
teo-tsirpanis
added
area-System.Linq.Expressions
and removed
area-Codegen-AOT-mono
area-NativeAOT-coreclr
labels
|
Tagging subscribers to this area: @cston Issue DetailsDescriptionThere is a difference in how
ExplanationNativeAOTThe issue was called out by @MichalStrehovsky noting that codepaths in
are not AOT friendly (reported here). For desktop platforms, NativeAOT fixes this by:
When it comes to iOS-like platforms, above is not true. When
MonoAOTAs for MonoAOT is concerned, there were several issues reported against the
includes generating code for a lot of generic delegates and Mono tries its best to support these, but at what cost? It is true that choosing delegates to implement fast invocation of methods should have better performance, but in case with Mono and delegates over value types, the compiler will generate Risk assessmentPros
Cons
ProposalHere is the list of tasks which should resolve all the reported issues around
Thanks to @MichalStrehovsky @vargaz @rolfbjarne for helping to identify these issues.
|
Universal shared generics wouldn't help with the size here - even on .NET Native, we only used universal shared code for the absolutely most dynamic scenarios (
We could also view it as a trimmer bug though - when we're running library level trimming (the way we do when we build the dotnet/runtime repo) and the library contains an embedded substitution manifest for something, maybe ILLinker should be smart enough to know that the substitution could replace the IL at app build time and maybe it shouldn't inline the value it sees in IL right now. The problem is really that the library trimming is generating a broken assembly (the warning we see at app build time |
As far as I know we want both behaviors. For example, for IntPtr.Size we want to inline the value at library build time, so that we can remove x86 code from x64 assembly. But in this case we would want to avoid inlining it. We would have to add some kind of hint to differentiate the two cases. For example we could add support for reading the |
|
For the IntPtr.Size case, the substitution is unconditional - we can inline at library build time because the value cannot change. I'm thinking about only skipping it for things that have a conditional substitution conditioned on some feature. Would that be still problematic? It feels like that's something we should be never inlining during libs build. |
|
You're right - but that would probably mean the feature switch definition is wrong in a way - library builds should basically never define any feature switch values - for exactly this reason. |
|
I see - the problem is that the code hardcodes a return value, the feature switch doesn't kick in at all (and linker doesn't consider substitutions which didn't apply due to feature switch values). Why don't we read this from AppContext just like other feature switches? It could still default to |
|
Why not to unify |
Doing this for |
|
CanEmitObjectArrayDelegate is going to crash on mono because it doesn't implement the private CoreLib API the else paths rely on. We can certainly make it not constant to work around the trimming limitations but there is no point to make this user overridable with appcontext because there's only one right answer per runtime configuration right now. |
I don't understand this. For example, How is |
|
It relies on a private API that only exists in NativeAOT CoreLib right now. |
|
FYI - a related discussion here is #81803 (comment). It's been on my backlog to get that fixed in .NET 8. |
I don't see fixing that as likely to help in this situation - we're unlikely to move the Reflection.Emit part to corelib as part of this fix (due to Linq.Expressions being archived). So we'd still need something to tell whether to call secret CoreLib APIs or do reflection emit. We have multiple options:
|
For Apple devices, we don't care about size when not trimming (we never have). |
In that case I would probably recommend removing the special build of Linq.Expressions for iOS-like platforms completely and use the same assembly everywhere. It should be possible to get the same results with feature switches and trimming. From what I see, we currently do two special things or iOS-like:
Instead, we could expose the option to remove Ref.Emit backend publicly (doc on how to do that here: https://github.com/dotnet/runtime/blob/e685ff9e0e1a57480df05abda6be135b72c8cbe6/docs/workflow/trimming/feature-switches.md) and have the Xamarin SDK turn the public flag on to remove it when trimming. |
Note that would also allow us to fix #80398. |
Is the |
The private API is to support creating an instance of a "magic" kind of delegate - given the type of the delegate to create |
|
@vargaz @LeVladIonescu is the functionality from Michal's previous comment:
covered with: #83329 ? |
|
Don't think so, thats a different issue. |
|
Considering the risks of changing the ILLink constant propagation to take into account definitions from the substitution file and changing the MonoAOT behaviour to match NativeAOT, would it make sense to instead:
This way we would:
As a follow-up we will investigate and work on enabling MonoAOT to have the support for Would this make sense? |
What are the cons of following this approach? It would align the "iDevice" build with the rest of the platforms (all other builds have I understand that it will result in a larger untrimmed assembly. But we should measure how much size are we actually talking about. Is it just a few KBs? And my understanding is that all "iDevice" apps will end up being trimmed at the app level anyway - so the resulting app sizes wouldn't be affected. |
It will be a size regression when not trimming because we'll no longer remove Ref.Emit-based expressions at library build time, but per #87924 (comment) it should not be a concern. We'll still trim it at app build time when trimming the app. |
|
I have measured the
Results
NOTE: The diffs presented above are just for performing measurements
|
|
I think |
|
@MichalStrehovsky as #88539 got merged in, should we open an issue to change how the private NativeAOT feature switches are handled: There are two problems with this:
|
Yep, looks like these
I think this one would be fixable by replacing the <ItemGroup>
<RuntimeHostConfigurationOption Include="System.Linq.Expressions.CanEmitObjectArrayDelegate"
Value="false"
Trim="true" />
</ItemGroup>I think if we put this into the NativeAOT targets, it would affect both ILLink and NativeAOT. I wouldn't pull this into a feature switch one can control externally - there's only one correct value for this and there's little reason to make it configurable. If this works, could you submit a PR? |
|
Sure, I will try it out and ping you. |
ivanpovazan
changed the title
Linq.Expressions.dll is built for iOS-like platformsSystem.Linq.Expressions.dll is supported on iOS-like platforms
|
I am closing this issue as completed as all the identified work has been done and adequate tracking issues were created. |
ghost
locked as resolved and limited conversation to collaborators
Description
There is a difference in how
System.Linq.Expressions.dllis built for iOS-like and other desktop/mobile platforms, which is observable here:runtime/eng/illink.targets
Line 226 in 2aa0c9b
runtime/src/libraries/System.Linq.Expressions/src/System.Linq.Expressions.csproj
Line 19 in 2aa0c9b
Up until now this difference was not noticeable, however with enabling support for iOS-like platforms with NativeAOT we started experiencing:
Linq.Expressionstests with NativeAOT on iOS-like platforms: https://github.com/dotnet/runtime/pull/87260/files#diff-4422ce02e9237f4f8e6021526506f87216778e497b33ce676872c1f70e1483c2R54System.Func`3delegate instances take up to 18% of accountable size of a MAUI iOS app with NativeAOT (the cause explained bellow)Additionally, there were several issues reported against MonoAOT, struggling with
Linq.Expressions.Interpreterreported here:Explanation
NativeAOT
The issue was called out by @MichalStrehovsky noting that codepaths in
Linq.Expressionslibrary controlled by:CanCompileToILCanEmitObjectArrayDelegateCanCreateArbitraryDelegatesare not AOT friendly (reported here).
For desktop platforms, NativeAOT fixes this by:
Linq.Expressions.dllruntime/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
Lines 271 to 273 in 2aa0c9b
When it comes to iOS-like platforms, above is not true. When
Linq.Expressionslibrary is built, constant propagation is enabled and control variables get removed during the library build.This further causes above-listed NativeAOT feature switches not to have any effect (fail to trim during app build), causing the AOT compilation to follow unsupported code paths which fail at runtime.
Examples:
Build warnings:
Test crash
runtime/src/tests/nativeaot/SmokeTests/UnitTests/Delegates.cs
Line 54 in 7b7d56f
MonoAOT
As for MonoAOT is concerned, there were several issues reported against the
Linq.Expressions.Interpretersupport. Some issues were fixed, some are still open, but in general following unfriendly AOT codepaths also hurts Mono.Example:
runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs
Line 35 in 2e764d6
runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.cs
Line 86 in 2e764d6
runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/CallInstruction.Generated.cs
Lines 44 to 154 in 2e764d6
includes generating code for a lot of generic delegates and Mono tries its best to support these, but at what cost?
It is true that choosing delegates to implement fast invocation of methods should have better performance, but in case with Mono and delegates over value types, the compiler will generate
GSHAREDVTmethods (generic sharing for value types) which are actually quite slow.On the other hand, NativeAOT does not have generic sharing for value types and generates instead all possible variations (causing the problem reported above 2) with a template MAUI app)
Risk assessment
Pros
Linq.Expressionson iOS-like platforms~30%smaller MAUI template app~2.5%smaller MAUI template app (the difference is way smaller compared to NativeAOT as Mono generates GSHAREDVT for fast invocation)Attempting to JIT compile method (wrapper delegate-invoke)users had to enable mono interpreter in their projects (UseInterpreter=true) to cover-up for missing methods during runtime, which also affects the application sizeCons
GSHAREDVTin these casesProposal
Here is the list of tasks which should resolve all the reported issues around
Linq.Expressionson iOS-like platformsSystem.Linq.Expression.dllbuild for all platforms #88723IsDynamicCodeSupportedfeature to false when using FullAOT xamarin/xamarin-macios#18340System.Linq.Expressionstests on iOS-like platforms #89168System.Linq.Expressionsprivate feature switches #89171Thanks to @MichalStrehovsky @vargaz @rolfbjarne for helping to identify these issues.
The text was updated successfully, but these errors were encountered: