Initial NET5/ILLink support #4669

radekdoulik · 2020-05-11T14:31:38Z

Implements #4707

With .NET5 we are switching from XA linker implemented as msbuild task to ILLink tool with custom steps provided in an assembly.

The new assembly with our custom steps is named Microsoft.Android.Sdk.ILLink and the source code is located in src/Microsoft.Android.Sdk.ILLink. It is used during build by ILLink tool.

The initial support is already able to link and run simple XA and XA/XF samples.

Future work

replace reflection usage in SetupStep Replace reflection usage when preparing linker pipeline #4709
audit reflection usage in our code Audit reflection usage in our assemblies #4708
add more customs steps - we are still missing MonoDroidMarkStep

Notes

the Profile API is not available in the linker public API. we work around it by adding src/Microsoft.Android.Sdk.ILLink/Profile.cs with missing pieces.
TypeDefinition.GetMethods extension method from Mono.Tuner.MethodBodyRocks is not public. It is very simple so we are inlining it.
the ILLink is now enabled in Release configuration defaults.
tests were updated to reference more packages, to not miss second level dependencies.

Initial results:

apkdiff output summary for HelloAndroid sample, comparing Debug and Release apk's

  -       7,300 Davik executables -2.34% (of 311,436)
  -  14,733,944 Assemblies -31.85% (of 46,265,064)
  -   2,492,452 Shared libraries -11.43% (of 21,803,164)
  -  15,528,456 Package size difference -28.70% (of 54,114,462)

Context:

_RunILLink target https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.targets
linker documentation https://github.com/mono/linker/tree/master/docs

src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.props

src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs

...marin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Linker.targets

src/Xamarin.Android.Linker/SetupStep.cs

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs

jonathanpeppers

It looks like some of the tests on CI are hitting:

Unrecognized command-line option: '--custom-data'

Do you know what .NET 5 version we need for this to work?

We can bump this number: https://github.com/xamarin/xamarin-android/blob/3f438e46d7b166a3a3ef54c9ffafb5f426760468/build-tools/automation/azure-pipelines.yaml#L53

We could maybe try 5.0.100-preview.5.20264.3, which is latest master here: https://github.com/dotnet/installer#build-status

radekdoulik · 2020-05-15T16:02:56Z

Do you know what .NET 5 version we need for this to work?

Locally I am using 5.0.100-preview.5.20229.12. I will try to bump it to lastest.

jonathanpeppers

We might have to wait until this lands: #4692

This fixes the change from netcoreapp5.0 -> net5.0.

radekdoulik · 2020-05-19T10:28:05Z

Indeed. dotnet tests are now failing with related error:

error XARSD7004: System.ArgumentException: `C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\5.0.0-preview.6.20264.1\ref\netcoreapp5.0` must be a directory! (Parameter 'frameworkDirectories')

jonathanpeppers · 2020-05-19T14:53:45Z

/azp run

azure-pipelines · 2020-05-19T14:54:07Z

Azure Pipelines successfully started running 1 pipeline(s).

jonpryor · 2020-05-20T00:37:20Z

DotNetBuild("android.21-arm",True) on Windows fails:

The assembly 'C:\A\vs2019xam00000L-1\_work\2\s\packages\microsoft.android.sdk\10.0.100-ci.pr.gh4669.53\targets\..\tools\Xamarin.Android.Linker.dll' specified for '--custom-step' option could not be found (TaskId:185)
18:23:35.119   1:7>C:\Program Files\dotnet\sdk\5.0.100-preview.6.20265.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.ILLink.targets(83,5): error MSB6006: "dotnet.exe" exited with code 1. [C:\A\vs2019xam00000L-1\_work\2\s\bin\TestRelease\temp\DotNetBuildandroid.21-armTrue\UnnamedProject.csproj]
                   Done executing task "ILLink" -- FAILED. (TaskId:185)

Based on the new `Microsoft.NET.ILLink` NuGet package, this is a start to what it would look like to use it for Xamarin.Android.

And use it in place of Mono.Tuner one, where needed for net5

Do not run `LinkAssemblies` task on NET5 Temporarily disable `_RemoveRegisterAttribute` target. It is crashing, presumably because the assemblies are in different location.

The step so far only installs `FixAbstractMethodsStep`. Parts of the code come from xamarin-macios repo. We might share these and move them to linker. Or enable more linker API to avoid getting the pipeline steps with reflection.

And re-enable `_RemoveRegisterAttribute` target, as the assemblies now end up in the right place

To avoid few #if's and make the code better readable

To avoid using Xamarin.Android.Linker namespace and introducing another `#if`'s

We don't need them anymore as we have the new extension method in that namespace

We still need `using Mono.Tuner;` for non NET5 version.

So that it is built

This can be enabled again when #4708 is resolved.

Which fell out in previous merge

To make the test build with linker

jonpryor · 2020-05-27T01:29:35Z

src/Xamarin.Android.Linker/Xamarin.Android.Linker.csproj

+  </PropertyGroup>
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.ILLink" Version="5.0.0-preview.3.20227.3" />
+    <ProjectReference Include="..\..\external\Java.Interop\src\Java.Interop.Export\Java.Interop.Export.csproj" />


I do not understand why we would have a @(ProjectReference) to Java.Interop.Export.dll. I don't see any mentions of types located within that assembly within this PR, e.g. neither JniSignature nor MarshalMemberBuilder make an appearance.

This is to have MarshalMemberBuilder.GetMarshalMethodName for TryGetMarshalMethod method.

jonpryor · 2020-05-27T01:40:14Z

Xamarin.Android.Build.Tasks.sln

-Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Xamarin.Android.Tools.AndroidSdk", "external\xamarin-android-tools\src\Xamarin.Android.Tools.AndroidSdk\Xamarin.Android.Tools.AndroidSdk.csproj", "{E34BCFA0-CAA4-412C-AA1C-75DB8D67D157}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Xamarin.Android.Tools.AndroidSdk", "external\xamarin-android-tools\src\Xamarin.Android.Tools.AndroidSdk\Xamarin.Android.Tools.AndroidSdk.csproj", "{E34BCFA0-CAA4-412C-AA1C-75DB8D67D157}"
+EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Xamarin.Android.Linker", "src\Xamarin.Android.Linker\Xamarin.Android.Linker.csproj", "{2A237E71-1FA3-409B-B13E-F6294932A5A9}"


I half wonder if this should be Microsoft.Android.Linker. :-/

Or even better Microsoft.Android.Sdk.ILLink ;-)

How many assemblies do we want running around? That's a rather fine-grained name…

jonpryor · 2020-05-27T01:41:47Z

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs

@@ -165,7 +166,7 @@ bool CheckInvokerType (TypeDefinition type, string name)

 		void PreserveInterfaceMethods (TypeDefinition type, TypeDefinition invoker)
 		{
-			foreach (var m in type.GetMethods ()) {
+			foreach (var m in type.Methods.Where (m => !m.IsConstructor)) {


Any specific reason for this change? The PR description is anemic.

This one comes from Mono.Tuner.MethodBodyRocks class, which is not part of illink public API and was simple enough to inline.

radekdoulik requested a review from jonathanpeppers May 11, 2020 14:31

radekdoulik requested review from dellis1972 and jonpryor as code owners May 11, 2020 14:31

jonathanpeppers reviewed May 11, 2020

View reviewed changes

dellis1972 reviewed May 12, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs Outdated Show resolved Hide resolved

radekdoulik force-pushed the net5-linker branch from 6d482af to bb3f0fa Compare May 14, 2020 12:36

jonathanpeppers reviewed May 14, 2020

View reviewed changes

jonathanpeppers reviewed May 18, 2020

View reviewed changes

jonathanpeppers and others added 17 commits May 20, 2020 13:23

[Xamarin.Android.Linker] exploring the new .NET linker

dbea5d5

Based on the new `Microsoft.NET.ILLink` NuGet package, this is a start to what it would look like to use it for Xamarin.Android.

#if around usage of SafeReadSymbols

b4f6f5c

Use newer illink nuget

47ad67d

Use the new linker logging iface (conditionaly)

49de381

Do not use Mono.Tuner sources in net5 linker

50e4fcc

Do not check for AppDomain usage on net5

9cb64f7

Use newer linker log API on net5

572fa0b

Use new Context.GetLoadedAssembly on net5

d6acfe5

Add Xamarin.Android.Linker Profile class

3df24dc

And use it in place of Mono.Tuner one, where needed for net5

Enable linker on NET5

573f854

Do not run `LinkAssemblies` task on NET5 Temporarily disable `_RemoveRegisterAttribute` target. It is crashing, presumably because the assemblies are in different location.

Run ILLink with our custom SetupStep

9ee06d1

The step so far only installs `FixAbstractMethodsStep`. Parts of the code come from xamarin-macios repo. We might share these and move them to linker. Or enable more linker API to avoid getting the pipeline steps with reflection.

Build more steps for net5

e61230f

Use the substeps, set the linker output directory

1cc7e6e

And re-enable `_RemoveRegisterAttribute` target, as the assemblies now end up in the right place

Clean old notes

029b6e6

Remove debug message

c396bec

Remove old comment

45c6574

Add proguard/r8 support

340d7e9

radekdoulik added 10 commits May 20, 2020 13:23

Add LinkContext.LogMessage extension method

6aae5e0

To avoid few #if's and make the code better readable

Put the extension in Mono.Linker namespace

9b5497f

To avoid using Xamarin.Android.Linker namespace and introducing another `#if`'s

Set MonoAndroidHelper.TargetFrameworkDirectories

3a0f853

Get rid of some of the #if's

3bafcc9

We don't need them anymore as we have the new extension method in that namespace

Removed a bit too much

915d90b

We still need `using Mono.Tuner;` for non NET5 version.

Restore the white space

06cc5ee

One more white space restore

ddfcc1f

One more using Mono.Tuner; missing.

cb80cad

Set output path to the location used for nuget

03550e4

Add Xamarin.Android.Linker.csproj to solution

42ba7a2

So that it is built

radekdoulik force-pushed the net5-linker branch from dd96ca3 to 42ba7a2 Compare May 20, 2020 14:31

radekdoulik mentioned this pull request May 21, 2020

Replace reflection usage when preparing linker pipeline #4709

Closed

radekdoulik added 5 commits May 21, 2020 19:52

Do not require 0 warning for Release

759622c

This can be enabled again when #4708 is resolved.

Merge branch 'master' into net5-linker

324e269

Add missing EndProject

466ac45

Which fell out in previous merge

Merge branch 'master' into net5-linker

f7cce5a

Additional packages for XForms 4.5 on NET5

fce7a06

To make the test build with linker

jonpryor reviewed May 27, 2020

View reviewed changes

Fix indentation

5c0413a

jonathanpeppers approved these changes Jun 1, 2020

View reviewed changes

Rename X.A.Linker to Microsoft.Android.Sdk.ILLink

aa27a90

radekdoulik merged commit f68f32a into master Jun 4, 2020

radekdoulik deleted the net5-linker branch June 4, 2020 12:03

brendanzagaeski added this to the In Preview: 11.1 (d16-8) milestone Aug 5, 2020

jonathanpeppers mentioned this pull request Sep 2, 2020

Initial ILLink support #4707

Closed

github-actions bot locked and limited conversation to collaborators Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial NET5/ILLink support #4669

Initial NET5/ILLink support #4669

radekdoulik commented May 11, 2020 •

edited

Loading

jonathanpeppers left a comment

radekdoulik commented May 15, 2020

jonathanpeppers left a comment

radekdoulik commented May 19, 2020

jonathanpeppers commented May 19, 2020

azure-pipelines bot commented May 19, 2020

jonpryor commented May 20, 2020

jonpryor May 27, 2020

radekdoulik Jun 3, 2020

jonpryor May 27, 2020

marek-safar May 27, 2020

jonpryor May 28, 2020

jonpryor May 27, 2020

radekdoulik Jun 3, 2020

Initial NET5/ILLink support #4669

Initial NET5/ILLink support #4669

Conversation

radekdoulik commented May 11, 2020 • edited Loading

jonathanpeppers left a comment

Choose a reason for hiding this comment

radekdoulik commented May 15, 2020

jonathanpeppers left a comment

Choose a reason for hiding this comment

radekdoulik commented May 19, 2020

jonathanpeppers commented May 19, 2020

azure-pipelines bot commented May 19, 2020

jonpryor commented May 20, 2020

jonpryor May 27, 2020

Choose a reason for hiding this comment

radekdoulik Jun 3, 2020

Choose a reason for hiding this comment

jonpryor May 27, 2020

Choose a reason for hiding this comment

marek-safar May 27, 2020

Choose a reason for hiding this comment

jonpryor May 28, 2020

Choose a reason for hiding this comment

jonpryor May 27, 2020

Choose a reason for hiding this comment

radekdoulik Jun 3, 2020

Choose a reason for hiding this comment

radekdoulik commented May 11, 2020 •

edited

Loading