Skip to content

Commit

Permalink
[illink] Remove custom types marking for types which XA does not own …
Browse files Browse the repository at this point in the history
…for netcore (#5354)

This is or will be handled by BCL libraries and XA tooling should try
not to interfere with more fine tuned handling done by BCL.

Simplifies `[Preserve]` attribute implementation and removes
old code to preserve serialization, which doesn't work
well with new .NET5/6 runtime libraries. (the old serialization
code: https://github.com/xamarin/xamarin-android/blob/master/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/ApplyPreserveAttribute.cs#L57-L82)

The serialization is currently [broken](dotnet/runtime#45559)
and will be fixed in runtime libs. Part of `CustomLinkDescriptionPreserve`
test is thus disabled.

Also the attribute removal is now handled in illink.

The serialization fix leads to great reduction of assemblies size in simple
XA test app by cca 1MB, apkdiff before/after:

    Summary:
      -   1,199,411 Assemblies -55.66% (of 2,154,984)
      -   1,206,333 Package size difference -13.37% (of 9,024,291)

Co-authored-by: Radek Doulik <radekdoulik@gmail.com>
  • Loading branch information
marek-safar and radekdoulik authored Dec 9, 2020
1 parent 97190b6 commit 89edafc
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@
namespace Microsoft.Android.Sdk.ILLink
{

public abstract class ApplyPreserveAttributeBase : BaseSubStep {

// set 'removeAttribute' to true if you want the preserved attribute to be removed from the final assembly
protected abstract bool IsPreservedAttribute (ICustomAttributeProvider provider, CustomAttribute attribute, out bool removeAttribute);
public class ApplyPreserveAttribute : BaseSubStep {

public override SubStepTargets Targets {
get {
Expand Down Expand Up @@ -66,7 +63,7 @@ public override void ProcessEvent (EventDefinition @event)

void MarkMethodIfPreserved (MethodDefinition method)
{
foreach (var attribute in GetPreserveAttributes (method))
foreach (var attribute in GetPreserveAttributes (method))
MarkMethod (method, attribute);
}

Expand Down Expand Up @@ -126,6 +123,9 @@ void PreserveUnconditional (IMetadataTokenProvider provider)

void TryApplyPreserveAttribute (TypeDefinition type)
{
if (!type.HasCustomAttributes)
return;

foreach (var attribute in GetPreserveAttributes (type)) {
Annotations.Mark (type);

Expand All @@ -138,28 +138,9 @@ void TryApplyPreserveAttribute (TypeDefinition type)
}
}

List<CustomAttribute> GetPreserveAttributes (ICustomAttributeProvider provider)
static IEnumerable<CustomAttribute> GetPreserveAttributes (ICustomAttributeProvider provider)
{
List<CustomAttribute> attrs = new List<CustomAttribute> ();

if (!provider.HasCustomAttributes)
return attrs;

var attributes = provider.CustomAttributes;

for (int i = attributes.Count - 1; i >= 0; i--) {
var attribute = attributes [i];

bool remote_attribute;
if (!IsPreservedAttribute (provider, attribute, out remote_attribute))
continue;

attrs.Add (attribute);
if (remote_attribute)
attributes.RemoveAt (i);
}

return attrs;
return provider.CustomAttributes.Where (a => a.Constructor.DeclaringType.Name == "PreserveAttribute");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\LinkerOptions.cs" />

<!--Steps that are upstreamed, these are OK to remove-->
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\ApplyPreserveAttribute.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\OutputStepWithTimestamps.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveCode.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveDynamicTypes.cs" />
Expand All @@ -36,6 +37,7 @@
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveRuntimeSerialization.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveTlsProvider.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveTypeConverters.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\RemoveAttributes.cs" />

<!--TODO: Subclasses MarkStep-->
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\MonoDroidMarkStep.cs" />
Expand Down
1 change: 0 additions & 1 deletion src/Microsoft.Android.Sdk.ILLink/SetupStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ protected override void Process ()
subSteps2.Add (new PreserveApplications ());
subSteps2.Add (new PreserveRegistrations (cache));
subSteps2.Add (new PreserveJavaInterfaces ());
subSteps2.Add (new RemoveAttributes ());

InsertAfter (new FixAbstractMethodsStep (cache), "RemoveUnreachableBlocksStep");
InsertAfter (subSteps2, "RemoveUnreachableBlocksStep");
Expand Down
10 changes: 10 additions & 0 deletions src/Mono.Android/ILLink/ILLink.LinkAttributes.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<linker>
<assembly fullname="Mono.Android">
<type fullname="Android.Runtime.PreserveAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="Android.Runtime.IntDefinitionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
</linker>
2 changes: 2 additions & 0 deletions src/Mono.Android/Mono.Android.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@

<ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp3.1' ">
<ProjectReference Include="..\..\external\Java.Interop\src\Java.Interop\Java.Interop.csproj" />

<EmbeddedResource Include="ILLink/ILLink.LinkAttributes.xml" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,38 +85,14 @@ public void CheckIncludedAssemblies ()
var expectedFiles = Builder.UseDotNet ?
new [] {
"Java.Interop.dll",
"Microsoft.Win32.Primitives.dll",
"Mono.Android.dll",
"System.Collections.NonGeneric.dll",
"System.ComponentModel.Primitives.dll",
"System.Console.dll",
"System.Formats.Asn1.dll",
"System.IO.Compression.Brotli.dll",
"System.IO.Compression.dll",
"System.IO.FileSystem.dll",
"System.Linq.Expressions.dll",
"System.Net.Http.dll",
"System.Net.NameResolution.dll",
"System.Net.NetworkInformation.dll",
"System.Net.Primitives.dll",
"System.Net.Quic.dll",
"System.Net.Security.dll",
"System.Net.Sockets.dll",
"System.ObjectModel.dll",
"System.Private.Uri.dll",
"System.Private.Xml.dll",
"System.Private.Xml.Linq.dll",
"System.Runtime.CompilerServices.Unsafe.dll",
"System.Runtime.InteropServices.RuntimeInformation.dll",
"System.Runtime.Numerics.dll",
"System.Security.Cryptography.Encoding.dll",
"System.Security.Cryptography.OpenSsl.dll",
"System.Security.Cryptography.X509Certificates.dll",
"System.Runtime.Serialization.Primitives.dll",
"System.Security.Cryptography.Algorithms.dll",
"System.Security.Cryptography.Primitives.dll",
"System.Text.RegularExpressions.dll",
"System.Threading.Channels.dll",
"System.Private.CoreLib.dll",
"System.Collections.Concurrent.dll",
"System.Collections.dll",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,9 @@ protected override void OnCreate(Bundle bundle)
Android.Util.Log.Info(TAG, LinkTestLib.Bug21578.MulticastOption_ShouldNotBeStripped());
Android.Util.Log.Info(TAG, LinkTestLib.Bug21578.MulticastOption_ShouldNotBeStripped2());
Android.Util.Log.Info(TAG, LinkTestLib.Bug35195.AttemptCreateTable());
#if !NET
Android.Util.Log.Info(TAG, LinkTestLib.Bug36250.SerializeSearchRequestWithDictionary());

#endif
Android.Util.Log.Info(TAG, "All regression tests completed.");
}
}
Expand Down
17 changes: 11 additions & 6 deletions tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,6 @@ public class LinkModeFullClass {
return sr.ReadToEnd ();
},
},
new BuildItem.Source ("Bug36250.cs") {
TextContent = () => {
using (var sr = new StreamReader (typeof (InstallAndRunTests).Assembly.GetManifestResourceStream ("Xamarin.Android.Build.Tests.Resources.LinkDescTest.Bug36250.cs")))
return sr.ReadToEnd ();
},
},
new BuildItem.Source ("Bug35195.cs") {
TextContent = () => {
using (var sr = new StreamReader (typeof (InstallAndRunTests).Assembly.GetManifestResourceStream ("Xamarin.Android.Build.Tests.Resources.LinkDescTest.Bug35195.cs")))
Expand All @@ -345,6 +339,17 @@ public class LinkModeFullClass {
},
};

if (!Builder.UseDotNet) {
// DataContractSerializer is not trimming safe
// https://github.com/dotnet/runtime/issues/45559
lib2.Sources.Add (new BuildItem.Source ("Bug36250.cs") {
TextContent = () => {
using (var sr = new StreamReader (typeof (InstallAndRunTests).Assembly.GetManifestResourceStream ("Xamarin.Android.Build.Tests.Resources.LinkDescTest.Bug36250.cs")))
return sr.ReadToEnd ();
},
});
}

proj = new XamarinFormsAndroidApplicationProject () {
IsRelease = true,
AndroidLinkModeRelease = linkMode,
Expand Down
6 changes: 5 additions & 1 deletion tests/MSBuildDeviceIntegration/Tests/InstallTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,11 @@ public void InstallWithoutSharedRuntime ()
// "The Shared Runtime should not have been installed.");
var directorylist = GetContentFromAllOverrideDirectories (proj.PackageName);
StringAssert.Contains ($"{proj.ProjectName}.dll", directorylist, $"{proj.ProjectName}.dll should exist in the .__override__ directory.");
StringAssert.Contains ($"System.dll", directorylist, $"System.dll should exist in the .__override__ directory.");
if (Builder.UseDotNet)
StringAssert.Contains ($"System.Private.CoreLib.dll", directorylist, $"System.Private.CoreLib.dll should exist in the .__override__ directory.");
else
StringAssert.Contains ($"System.dll", directorylist, $"System.dll should exist in the .__override__ directory.");

StringAssert.Contains ($"Mono.Android.dll", directorylist, $"Mono.Android.dll should exist in the .__override__ directory.");
Assert.IsTrue (builder.Uninstall (proj), "unnstall should have succeeded.");
}
Expand Down

0 comments on commit 89edafc

Please sign in to comment.