Skip to content

[coreclr] Initial support of R2R builds #10007

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 21, 2025
Merged

[coreclr] Initial support of R2R builds #10007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9a3e300
Enable R2R builds
ivanpovazan Apr 4, 2025
772de8e
Add R2R build tests
ivanpovazan Apr 7, 2025
de98b73
Update src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Te…
ivanpovazan Apr 16, 2025
619186b
Use better verification method that app assembly in the apk has R2R i…
ivanpovazan Apr 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 59 additions & 4 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,8 @@ public void BuildBasicApplication ([Values (true, false)] bool isRelease, [Value
}

[Test]
public void BasicApplicationOtherRuntime ([Values (true, false)] bool isRelease)
public void BasicApplicationBuildCoreCLR ([Values (true, false)] bool isRelease)
{
// This test would fail, as it requires **our** updated runtime pack, which isn't currently created
// It is created in `src/native/native-clr.csproj` which isn't built atm.
Assert.Ignore ("CoreCLR support isn't fully enabled yet. This test will be enabled in a follow-up PR.");
var proj = new XamarinAndroidApplicationProject {
IsRelease = isRelease,
// Add locally downloaded CoreCLR packs
Expand All @@ -124,6 +121,64 @@ public void BasicApplicationOtherRuntime ([Values (true, false)] bool isRelease)
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
}

static object [] ReadyToRunConfigurationSource = new object [] {
new object[] {
/* isComposite */ true,
/* rid */ "android-x64"
},
new object[] {
/* isComposite */ false,
/* rid */ "android-x64"
},
new object[] {
/* isComposite */ true,
/* rid */ "android-arm64"
},
new object[] {
/* isComposite */ false,
/* rid */ "android-arm64"
}
};

[Test]
[TestCaseSource (nameof (ReadyToRunConfigurationSource))]
public void BasicApplicationPublishReadyToRun (bool isComposite, string rid)
{
var proj = new XamarinAndroidApplicationProject {
IsRelease = true,
// Add locally downloaded CoreCLR packs
ExtraNuGetConfigSources = {
Path.Combine (XABuildPaths.BuildOutputDirectory, "nuget-unsigned"),
}
};

proj.SetProperty ("RuntimeIdentifier", rid);
proj.SetProperty ("UseMonoRuntime", "false"); // Enables CoreCLR
proj.SetProperty ("_IsPublishing", "true"); // Make "dotnet build" act as "dotnet publish"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update this line for when $(PublishReadyToRun) is true:

android/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.DefaultProperties.targets

Line 34 in 747041d

<_IsPublishing Condition=" '$(_IsPublishing)' == '' and '$(_AndroidRuntime)' == 'NativeAOT' ">true</_IsPublishing>

I would expect dotnet build -c Release -p:PublishReadyToRun=true to work by default on Android.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make this as a temporary workaround for R2R as well.
However, on the long run we shouldn't force dotnet build -c Release -p:PublishReadyToRun=true to do publishing as that would not match the behavior on other platforms.
On desktop and on iOS, triggering NativeAOT builds only happens with dotnet publish , dotnet build uses "regular" default runtimes/builds -> CoreCLR desktop, Mono iOS

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for reference, MAUI on Windows uses PublishReadyToRun=true by default on Release builds since .NET 7:

If R2R is what makes the startup performance reasonable, I think we have to use it by default on Android as well? So, dotnet build -c Release on Android would use R2R?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That only applies if you are publishing your app through Visual Studio, or manually through dotnet publish.
dotnet build will not / should not run publish targets when PublishReadyToRun=true.

On a regular console app if you add PublishReadyToRun=true to the project and run dotnet build it will not run R2R AOT compiler. But if you run dotnet publish it will.

So to come back to my previous comment, we should not force _IsPublishing=true for NativeAOT nor R2R just so we would make
dotnet build -p:PublishAOT=true or
dotnet build -p:PublishReadyToRun=true to run/act like dotnet publish, it is against the behavior we have on other platforms (e.g., on macios we solved it with yet another property: https://github.com/dotnet/macios/blob/7cef45367161ca26d3a68e2e096639194ed4df1f/dotnet/targets/Xamarin.Shared.Sdk.props#L39)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is people can actually ship their app with dotnet build -c Release for Android today (and they are). There is currently no requirement to use dotnet publish on Android.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per our offline discussion, we will address this in the follow-up work of fully enabling R2R builds

proj.SetProperty ("PublishReadyToRun", "true"); // Enable R2R
proj.SetProperty ("AndroidEnableAssemblyCompression", "false");

if (isComposite)
proj.SetProperty ("PublishReadyToRunComposite", "true"); // Enable R2R composite

var b = CreateApkBuilder ();
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
Comment on lines +164 to +165
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test, is there a way to assert the .dll files inside the .apk has a ReadyToRun image inside? Is Mono.Cecil able to detect them? We already have some tests that use Cecil to assert build output.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could maybe just inspect whether assemblies from obj/Release/netX.0-android/android-X/R2R/ end up in the .apk? I am saying this because the SPC.dll we ship with CoreCLR already includes R2R image in it - even without enabling R2R builds at app build time.
I will look into it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be enough to compare the MVID of the obj/Release/netX.0-android/android-X/R2R/*.dll file and the one inside the .apk?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inspecting the .apk it self doesn't seem to be doable. As in CoreCLR builds we use assembly store and all assemblies get merged and converted into a single .so file that ends up in the .apk.
Do we have a way of reversing that process and extracting the .dll it self?

If not, we could potentially add a custom MSBuild target to the test project that simply prints out ResolvedUserAssemblies which go into GeneratePackageManagerJava, if those paths include obj/Release/netX.0-android/android-X/R2R/*.dll assemblies the test passes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the test, I think you could use:

android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs

Lines 281 to 290 in b54ec05

var apk = Path.Combine (Root, b.ProjectDirectory, proj.OutputPath, $"{proj.PackageName}-Signed.apk");
FileAssert.Exists (apk);
var helper = new ArchiveAssemblyHelper (apk, useAssemblyStore);
foreach (string abi in proj.GetRuntimeIdentifiersAsAbis ()) {
Assert.IsTrue (helper.Exists ($"assemblies/{abi}/{assemblyName}.dll"), $"{assemblyName}.dll should exist in apk!");
using var stream = helper.ReadEntry ($"assemblies/{assemblyName}.dll");
stream.Position = 0;
using var assembly = AssemblyDefinition.ReadAssembly (stream);

Then assembly you could either check the MVID or some flag that says there is a R2R image.


var assemblyName = proj.ProjectName;
var apk = Path.Combine (Root, b.ProjectDirectory, proj.OutputPath, rid, $"{proj.PackageName}-Signed.apk");
FileAssert.Exists (apk);

var helper = new ArchiveAssemblyHelper (apk, true);
var abi = MonoAndroidHelper.RidToAbi (rid);
Assert.IsTrue (helper.Exists ($"assemblies/{abi}/{assemblyName}.dll"), $"{assemblyName}.dll should exist in apk!");

using var stream = helper.ReadEntry ($"assemblies/{assemblyName}.dll");
stream.Position = 0;
using var peReader = new System.Reflection.PortableExecutable.PEReader (stream);
Assert.IsTrue (peReader.PEHeaders.CorHeader.ManagedNativeHeaderDirectory.Size > 0,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathanpeppers I used the recommendation from: dotnet/runtime#38440 (comment) to verify that the app main assembly that gets packed into the .apk has R2R image in it.

$"ReadyToRun image not found in {assemblyName}.dll! ManagedNativeHeaderDirectory should not be empty!");
}

[Test]
public void NativeAOT ()
{
Expand Down
9 changes: 3 additions & 6 deletions src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,13 +342,10 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
<AndroidUseAssemblyStore Condition=" '$(AndroidUseAssemblyStore)' == '' ">true</AndroidUseAssemblyStore>
<AndroidAotEnableLazyLoad Condition=" '$(AndroidAotEnableLazyLoad)' == '' And '$(AotAssemblies)' == 'true' And '$(AndroidIncludeDebugSymbols)' != 'true' ">True</AndroidAotEnableLazyLoad>
<AndroidEnableMarshalMethods Condition=" '$(AndroidEnableMarshalMethods)' == '' and ('$(UsingMicrosoftNETSdkRazor)' == 'true') ">False</AndroidEnableMarshalMethods>
<AndroidEnableMarshalMethods Condition=" '$(AndroidEnableMarshalMethods)' == '' and '$(_AndroidRuntime)' != 'NativeAOT' ">True</AndroidEnableMarshalMethods>
<AndroidEnableMarshalMethods Condition=" '$(AndroidEnableMarshalMethods)' == '' and '$(_AndroidRuntime)' != 'NativeAOT' and '$(PublishReadyToRun)' != 'true' ">True</AndroidEnableMarshalMethods>
<!-- NOTE: temporarily disable for NativeAOT for now, to get build passing -->
<AndroidEnableMarshalMethods Condition=" '$(AndroidEnableMarshalMethods)' == '' and '$(_AndroidRuntime)' == 'NativeAOT' ">False</AndroidEnableMarshalMethods>
<AndroidEnableMarshalMethods Condition=" '$(AndroidEnableMarshalMethods)' == '' and ('$(_AndroidRuntime)' == 'NativeAOT' or '$(PublishReadyToRun)' == 'true') ">False</AndroidEnableMarshalMethods>

<!-- NOTE: temporarily disable for CoreCLR for now, until we have an implementation that works
with the new runtime -->
<AndroidEnableMarshalMethods Condition=" '$(AndroidEnableMarshalMethods)' == '' and '$(_AndroidRuntime)' == 'CoreCLR' ">False</AndroidEnableMarshalMethods>
<_AndroidUseMarshalMethods Condition=" '$(AndroidIncludeDebugSymbols)' == 'True' ">False</_AndroidUseMarshalMethods>
<_AndroidUseMarshalMethods Condition=" '$(AndroidIncludeDebugSymbols)' != 'True' ">$(AndroidEnableMarshalMethods)</_AndroidUseMarshalMethods>

Expand Down Expand Up @@ -2062,7 +2059,7 @@ because xbuild doesn't support framework reference assemblies.

<!-- Shrink Mono.Android.dll by removing attribute only needed for GenerateJavaStubs -->
<RemoveRegisterAttribute
Condition="'$(AndroidLinkMode)' != 'None' and '$(AndroidIncludeDebugSymbols)' != 'true' and '$(AndroidStripILAfterAOT)' != 'true' and '$(_AndroidRuntime)' != 'NativeAOT' "
Condition="'$(AndroidLinkMode)' != 'None' and '$(AndroidIncludeDebugSymbols)' != 'true' and '$(AndroidStripILAfterAOT)' != 'true' and '$(_AndroidRuntime)' != 'NativeAOT' and '$(PublishReadyToRun)' != 'true' "
ShrunkFrameworkAssemblies="@(_ShrunkAssemblies)" />

<MakeDir Directories="$(MonoAndroidIntermediateAssemblyDir)shrunk" />
Expand Down
Loading