[libs][iOS] Unify System.Linq.Expression.dll build for all platforms

[ivanpovazan]

Description

This PR removes special builds of System.Linq.Expression.dll for iOS-like platforms enabling us to ship a single assembly for all platforms, and to have better compatibility between our AOT engines.

These changes contribute towards fixing how the library is built for iOS-like platforms preventing conditional variables like: CanEmitObjectArrayDelegate to be removed from the library during library build time, which are required to be present for AOT compilation (during app deployment) with NativeAOT, as discussed in: #87924

Changes

The following changes are introduced:

• Library build-time substitution: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Linq.Expressions/src/ILLink/ILLink.Substitutions.IsInterpreting.LibraryBuild.xml has been removed
• Library build-time constant propagation is disabled

Measurements

Unifying how the library is built for all platforms affects the library size, which can be seen from the following measurements:

System.Linq.Expressions.dll	Size (b)	diff (b)	diff (%)
main	449024	NaN	NaN
this PR	562176	113152	25,2%

The increase in size of 25% comes from the fact that Ref.Emit code paths are now included in the assembly. However, this does not cause any size regressions for applications built for iOS-like platforms, because during app deployment the unsupported code paths (like Ref.Emit) will be removed during trimming due to:

runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/LambdaExpression.cs

Line 29 in d361507

public static bool CanCompileToIL => RuntimeFeature.IsDynamicCodeSupported;

as IsDynamicCodeSupported=false on these platforms.

Summary

This PR affects the following:

• the size of the shipped System.Linq.Expression.dll for iOS-like platforms is increased by ~113Kb (~25%)
• the fullAOT + UseInterpreter=true configurations with Mono (when interpreter is used as a fallback on iOS-like platforms) are now interpreting Ref.Emit generated code instead of using Reflection
  • the size of deployed apps in this configuration is also increased by keeping Ref.Emit backend (which should not be a problem as we do not recommend this setup for app deployment in the first place)

The apps deployed with standard Mono fullAOT configurations for iOS-like platforms should not be affected by this change once: xamarin/xamarin-macios#18555 gets merged in.

[ghost]

Tagging subscribers to this area: @cston
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This PR contributes to fixing how System.Linq.Expression.dll is built for iOS-like platforms preventing conditional variables like: CanEmitObjectArrayDelegate to be removed from the library, which are required to be present for AOT compilation (during app deployment) with NativeAOT, as discussed in: #87924

Disabling constant propagation during library build time minimally affects the library size, which can be seen from the following measurements:

System.Linq.Expressions.dll	Size (b)	diff (b)	diff (%)
main	449024	NaN	NaN
this PR	449536	512	0,11%

Furthermore, since constant propagation is always enabled during trimming / app deployment, the end effect regarding final app size is unchanged.

Author:	ivanpovazan
Assignees:	ivanpovazan
Labels:	area-System.Linq.Expressions
Milestone:	-

[ivanpovazan]

/cc: @steveisok @akoeplinger

[ivanpovazan]

@kotlarmilos once this gets merged in, the smoke tests for Linq.Expressions with NativeAOT on iOS-like platforms should be enabled again.

[MichalStrehovsky]

If I'm reading this right the only thing special left for iDevices is this part:

https://github.com/dotnet/runtime/blob/6bcc188bcef478be5efff61f54f7a61bb487dfa0/src/libraries/System.Linq.Expressions/src/System.Linq.Expressions.csproj#L13C33-L13C33

Does this do anything with IPConstProp disabled? It's a substitution that only applies during library build.

If it doesn't do anything, we should delete the special iDevices build and basically roll back the csproj part of #54970.

[ivanpovazan]

If I'm reading this right the only thing special left for iDevices is this part:

https://github.com/dotnet/runtime/blob/6bcc188bcef478be5efff61f54f7a61bb487dfa0/src/libraries/System.Linq.Expressions/src/System.Linq.Expressions.csproj#L13C33-L13C33

Does this do anything with IPConstProp disabled? It's a substitution that only applies during library build.

If it doesn't do anything, we should delete the special iDevices build and basically roll back the csproj part of #54970.

It still has an effect, even with IPConstProp disabled it will remove the Ref.Emit pieces from the library during the library build. Basically, we always ship System.Linq.Expressions.dll without Ref.Emit for iOS/tvOS/Catalyst.

I agree that essentially during app deployment the same thing would happen, but I suppose it was decided not to ship the code which will never going to be supported on such platforms. Additionally, as I posted in: #87924 (comment) excluding the library build time substitution will also bump the size of the assembly by ~100kb (which affects nuget/workload size - not by much though :) )

@steveisok what do you think?

[steveisok]

I'm fine with carrying extra size. An extra 100k is not significant.

[ivanpovazan]

Trying to unify the library build got me thinking that for Mono:

runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/LambdaExpression.cs

Line 29 in d361507

public static bool CanCompileToIL => RuntimeFeature.IsDynamicCodeSupported;

should be instead:

public static bool CanCompileToIL => RuntimeFeature.IsDynamicCodeCompiled; 

Because when Mono interpreter is enabled the IsDynamicCodeSupported=true but IsDynamicCodeCompiled=false. This wasn't an issue before because CanCompileToIL was substituted during library build time.

[eerhardt]

Suggested change

	<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppCurrent)-ios;$(NetCoreAppCurrent)-tvos;$(NetCoreAppCurrent)-maccatalyst</TargetFrameworks>
	<TargetFramework>$(NetCoreAppCurrent)</TargetFramework>

We don't need to multi-target this project anymore since we no longer have differences between these builds.

[eerhardt]

public static bool CanCompileToIL => RuntimeFeature.IsDynamicCodeCompiled;

This is always a tough decision. I don't think there is a straight forward answer. In some cases in Mono (for example WASM), I found that using interpreted IL was faster than using Reflection. See the discussion and numbers in #38693 (comment).

So the answer is "it depends". It depends if Mono interpreter (the only thing today where these bools will be different) generating IL through Ref.Emit and then interpreting it is faster/better than using the Reflection fallback path. For System.Text.Json Serializer on WASM, we found that Ref.Emit was faster, so we still use it even though IsDynamicCodeCompiled==false.

It would be good to test/measure it.

[MichalStrehovsky]

There are related discussions here:

#17973 - this is about ability of third-party code to query whether LINQ expressions are going to be compiled. Some libraries like Newtonsoft use LINQ expressions to make high performance serializers (hoping the expression is compiled to IL). But if it's interpreted, they're better off using reflection. They need a way to find out at runtime whether this is going to be compiled or interpreted.

#79726 - this is about exposing it as a public feature switch that people can set to get more trimming. For example, the SDK could set this feature switch to trim Ref.Emit backend when OptimizationPreference=Size. We currently only respect OptimizationPreference in Native AOT, but the name of this setting has been purposefully made non-AOT specific, so one could make this strip the Ref.Emit backend when PublishTrimmed is active. (OptimizationPreference can make semantics-preserving optimizations like this automatically. It must not do semantics-changing optimizations.)

I don't think we should change this to public static bool CanCompileToIL => RuntimeFeature.IsDynamicCodeCompiled;, but the above issues might be worth prioritizing higher (we don't need it for 8.0, but might be nice for 9.0).

[ivanpovazan]

There is another scenario which is affected with this change.
That is: Mono full AOT but with UseInterpreter=true, which is used in cases when full AOT experience is not possible and Mono interpreter is used as a fallback even on iOS-like platforms. Prior to this change Ref.Emit was not present in the library, while with this change and in the mentioned setup, Mono will start interpreting the Ref.Emit code, instead of using reflection.

[ivanpovazan]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ivanpovazan]

/azp run runtime-maccatalyst,runtime-ioslikesimulator

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[kotlarmilos]

Some of the test failures seem related:

• net8.0-iossimulator-Release-x64-Mono_Release-OSX.1200.Amd64.Open
• net8.0-maccatalyst-Release-x64-Mono_Release-OSX.1200.Amd64.Open
• net8.0-tvossimulator-Release-x64-Mono_Release-OSX.1200.Amd64.Open

are failing with:

System.Linq.Expressions.Tests[45078:233755] ((null) error) Ran out of trampolines of type 1 in '/private/tmp/helix/working/B4140A14/w/B137092D/e/System.Linq.Expressions.Tests.app/Contents/Resources/System.Private.CoreLib.dll' (limit 50000)

Check out #85759.

This has been fixed on iOS devices (allow an infinite number of trampolines) but is not ported to all platforms like the ios simulators. I think you can just bump them.

[ivanpovazan]

Yeah I've just bumped them before the last run. Thanks!

[ivanpovazan]

/azp run runtime-maccatalyst

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ivanpovazan]

After some testing on maccatalyst-x64 we need to set upper limit of nrgctx-trampolines=110000 to support this new configuration.

At the bottom of the following log it can be seen that 109264 are needed for the test in question to pass:
https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-89078-merge-c55e11aed3fc47c1ab/System.Linq.Expressions.Tests/1/System.Linq.Expressions.Tests.log?helixlogtype=result

[ivanpovazan]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[MichalStrehovsky]

Some libraries like Newtonsoft use LINQ expressions to make high performance serializers (hoping the expression is compiled to IL). But if it's interpreted, they're better off using reflection.

See my comment above about "it depends". I found that But if it's interpreted, they're better off using reflection. is not always true.

Reflection is always faster than interpreted LINQ (I did not mean Ref.Emit interpreter, I meant the LINQ preferInterpretation: true). Interpreted LINQ is going to call into the exact same reflection APIs the user could have called, but only after doing a bunch of extra interpreter work.

[ivanpovazan]

Failures are not related.