From a9706b6ef0429250ecaf1e500d77cd19e94e2eb5 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Wed, 29 May 2024 01:26:31 +0200 Subject: [PATCH] [Xamarin.Android.Build.Tasks] LLVM Marshal Methods by Default, Take 2 (#8925) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: 68368189d67c46ddbfed4e90e622f635c4aff11e Context: 8bc7a3e84f95e70fe12790ac31ecd97957771cb2 Commit 8bc7a3e8 enabled LLVM Marshal Methods by default, and commit 68368189 *disabled* LLVM Marshal Methods by default, because it appeared to contribute to hangs in MAUI+Blazor apps. After lots of additional cleanup, refactoring, and investigation… we *still* don't understand why MAUI+Blazor apps hang, and we want the app time startup improvements that LLVM Marshal Methods allow. Square this circle by enabling LLVM Marshal Methods by default, *unless* Blazor is detected, in which case LLVM Marshal Methods will be *disabled* automatically. ~~ App startup time improvements ~~ Measurements are across 50 runs if a test app, with the fastest and slowest runs removed from the set. * Displayed time * Best result: **15.73%** faster (default settings *without* ProfiledAOT and compression) * Default settings result: **12.66%** faster. * Native to managed transition * Best result: **1.31%** faster * Default settings result: **0.35%** slower * Total managed runtime init * Best result: **2.55%** faster (default settings without ProfiledAOT) * Default settings result: **0.71%** faster ~~ Build Target Interdependencies ~~ While developing this change, we ran into an odd bug, via the `InstallAndRunTests.EnableAndroidStripILAfterAOT(false)` test: % dotnet new android % dotnet build -c Release -v:diag -p:AndroidEnableMarshalMethods=true \ -p:AndroidStripILAfterAOT=true -p:AndroidEnableProfiledAot=false > b.txt % dotnet build -c Release -v:diag -p:AndroidEnableMarshalMethods=true \ -p:AndroidStripILAfterAOT=true -p:AndroidEnableProfiledAot=false -t:Install > i.txt % dotnet build -c Release -t:StartAndroidActivity The app immediately crashes on startup; from `adb logcat`: F mono-rt : [ERROR] FATAL UNHANDLED EXCEPTION: System.InvalidProgramException: Invalid IL code in Android.Runtime.JNIEnvInit:Initialize (Android.Runtime.JNIEnvInit/JnienvInitializeArgs*): method body is empty. The reason fo the crash is that during the first `dotnet build` invocation, the marshal methods classifier works as it should and, consequently, the marshal methods rewriter *modifies the assemblies*: removes connector methods, generates wrapper methods, etc etc. As part of this, the `_GenerateJavaStubs` target eventually creates a stamp file which it then uses to decide whether all the inputs are up to date with regards to their corresponding outputs. However, at the end of the first `dotnet build`, `ILStrip` runs which modifies assemblies *after* marshal methods processing is done. When we run the `dotnet build -t:Install` command, the the `_GenerateJavaStubs` target runs *again* as MSBuild notices that the stamp file is older than some of the inputs: assemblies modified by `ILStrip` after `_GenerateJavaStubs` created its stamp file. This causes the target to run completely: Target "_GenerateJavaStubs: (TargetId:337)" in file "/usr/local/share/dotnet/packs/Microsoft.Android.Sdk.Darwin/34.0.95/tools/Xamarin.Android.Common.targets" from project "…/gxa-8925.csproj" (target "_GeneratePackageManagerJava" depends on it): Building target "_GenerateJavaStubs" completely. Input file "obj/Release/net8.0-android/android-arm/linked/gxa-8925.dll" is newer than output file "obj/Release/net8.0-android/stamp/_GenerateJavaStubs.stamp". which causes the whole marshal methods processing to be initiated again, but this time the connector methods and anything else the classifier looks for aren't there, because they were previously removed by the rewriter during the first `dotnet build` command. This, in turn, causes the classifier to decide that types need to be registered dynamically, but they can't because the connector methods which are required to create delegates are no longer there and we get a runtime crash instead. The solution is to update the `_GenerateJavaStubs` stamp file after `ILStrip` is done, within the `_AndroidAotCompilation` target, if it modified any assemblies. One problem with that is that the path to the stamp file's directory is different when `_GenerateJavaStubs` runs during the build phase, and when AOT and `ILStrip` run. In the former case, the stamp file is created in `obj/${Configuration}/stamp` directory, in the latter case in `obj/${Configuration}/android-ARCH/stamp` directory, but *both* locations are initialized in the same spot: at the top of `Xamarin.Android.Common.targets`. In effect, after `ILStrip` runs, we don't really know where `_GenerateJavaStubs` created its stamp file. Workaround this by taking advantage of the fact that we know where the stamp directories are in relation to each other. It's a hack, but if the relation is somehow broken, the `InstallAndRunTests.EnableAndroidStripILAfterAOT(false)` test will break again. --- .../targets/Microsoft.Android.Sdk.Aot.targets | 24 ++++ .../BuildReleaseArm64SimpleDotNet.apkdesc | 28 ++--- .../BuildReleaseArm64XFormsDotNet.apkdesc | 113 +++++++++--------- .../Xamarin.Android.Common.targets | 3 +- 4 files changed, 95 insertions(+), 73 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Aot.targets b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Aot.targets index 40446704aff..7b9ebac3621 100644 --- a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Aot.targets +++ b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Aot.targets @@ -143,6 +143,30 @@ They run in a context of an inner build with a single $(RuntimeIdentifier). SourceFiles="@(_ILStripUpdatedAssemblies)" DestinationFiles="@(_ILStripUpdatedAssemblies->'%(UntrimmedAssemblyFilePath)')" /> + + <_UpdateStamp Include="@(_ILStripUpdatedAssemblies)" Condition=" '$(AndroidStripILAfterAOT)' == 'true' and '%(_ILStripUpdatedAssemblies.ILStripped)' == 'true' " /> + + + + + <_StampDir>$(_AndroidStampDirectory)\..\..\stamp + + + false true True - False + False + True <_AndroidUseMarshalMethods Condition=" '$(AndroidIncludeDebugSymbols)' == 'True' ">False <_AndroidUseMarshalMethods Condition=" '$(AndroidIncludeDebugSymbols)' != 'True' ">$(AndroidEnableMarshalMethods)