From 09bce94b723d2b52fc98ce6a3dc5f6a3a19d4b2a Mon Sep 17 00:00:00 2001 From: Sebastien Pouliot Date: Wed, 23 Jun 2021 16:37:16 -0400 Subject: [PATCH 1/3] [WIP][dotnet][linker] Enable the sealer optimization when (by default) * the interpreter is not enabled (since new code might subclass or override the members analyzed at build time) * building for release Reverts https://github.com/xamarin/xamarin-macios/commit/c56b893b68811ffdd1065aa52e8589b72df49fb4 Fix https://github.com/xamarin/xamarin-macios/issues/9573 Work-in-progress: * should be made easier to enable/disable (from user projects) * the above conditions should be the default (if nothing is specified) --- dotnet/targets/Xamarin.Shared.Sdk.targets | 3 +++ tests/linker/ios/link all/SealerTest.cs | 9 --------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/dotnet/targets/Xamarin.Shared.Sdk.targets b/dotnet/targets/Xamarin.Shared.Sdk.targets index 20ad29ca7cb7..8e36bd9ec858 100644 --- a/dotnet/targets/Xamarin.Shared.Sdk.targets +++ b/dotnet/targets/Xamarin.Shared.Sdk.targets @@ -302,6 +302,9 @@ --> <_ExtraTrimmerArgs>$(_ExtraTrimmerArgs) --disable-opt unusedtypechecks + + <_ExtraTrimmerArgs Condition="'$(_BundlerDebug)' != 'true' And '$(MtouchInterpreter)' == '' And '$(_PlatformName)' != 'macOS'">$(_ExtraTrimmerArgs) --enable-opt sealer + false diff --git a/tests/linker/ios/link all/SealerTest.cs b/tests/linker/ios/link all/SealerTest.cs index 1ebe75909078..297600627be0 100644 --- a/tests/linker/ios/link all/SealerTest.cs +++ b/tests/linker/ios/link all/SealerTest.cs @@ -36,9 +36,6 @@ public class Subclass : Base, Interface { [Preserve (AllMembers = true)] public class SealerTest { -#if !DEBUG && NET - [Ignore ("https://github.com/xamarin/xamarin-macios/issues/9573")] -#endif [Test] public void Sealed () { @@ -62,9 +59,6 @@ public void Sealed () #endif } -#if !DEBUG && NET - [Ignore ("https://github.com/xamarin/xamarin-macios/issues/9573")] -#endif [Test] public void Final () { @@ -85,9 +79,6 @@ public void Final () #endif } -#if !DEBUG && NET - [Ignore ("https://github.com/xamarin/xamarin-macios/issues/9573")] -#endif [Test] public void Virtual () { From f1de3c56762a2786bc9d56f8cbebf542ffac07da Mon Sep 17 00:00:00 2001 From: Sebastien Pouliot Date: Wed, 4 Aug 2021 17:20:17 -0400 Subject: [PATCH 2/3] Fix test and update comment Even if possible (in metadata) there is no point in setting `final` on a method if we remove `virtual`. This match ILLink version of the sealer and makes the same test pass on both. --- dotnet/targets/Xamarin.Shared.Sdk.targets | 2 +- tests/linker/ios/link all/SealerTest.cs | 2 +- tools/linker/MonoTouch.Tuner/SealerSubStep.cs | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dotnet/targets/Xamarin.Shared.Sdk.targets b/dotnet/targets/Xamarin.Shared.Sdk.targets index 89ae91813748..68745580aa9b 100644 --- a/dotnet/targets/Xamarin.Shared.Sdk.targets +++ b/dotnet/targets/Xamarin.Shared.Sdk.targets @@ -464,7 +464,7 @@ --> <_ExtraTrimmerArgs>$(_ExtraTrimmerArgs) --disable-opt unusedtypechecks - + <_ExtraTrimmerArgs Condition="'$(_BundlerDebug)' != 'true' And '$(MtouchInterpreter)' == '' And '$(_PlatformName)' != 'macOS'">$(_ExtraTrimmerArgs) --enable-opt sealer diff --git a/tests/linker/ios/link all/SealerTest.cs b/tests/linker/ios/link all/SealerTest.cs index 297600627be0..571e3b669d82 100644 --- a/tests/linker/ios/link all/SealerTest.cs +++ b/tests/linker/ios/link all/SealerTest.cs @@ -75,7 +75,7 @@ public void Final () // but it can be optimized / sealed as nothing else is (or can) overrides it Assert.True (a.IsFinal, "A"); Assert.True (b.IsFinal, "B"); - Assert.True (c.IsFinal, "C"); + Assert.False (c.IsFinal, "C"); // devirtualized #endif } diff --git a/tools/linker/MonoTouch.Tuner/SealerSubStep.cs b/tools/linker/MonoTouch.Tuner/SealerSubStep.cs index 51dc844b5233..645c94d1b7f1 100644 --- a/tools/linker/MonoTouch.Tuner/SealerSubStep.cs +++ b/tools/linker/MonoTouch.Tuner/SealerSubStep.cs @@ -102,6 +102,7 @@ protected override void Process (TypeDefinition type) // look if this method is an override to existing _marked_ methods if (!AreMarked (bases)) { method.IsVirtual = false; + method.IsFinal = false; // since it's not virtual anymore #if DEBUG Console.WriteLine ("Devirtualize {0} ({1})", method, ++devirtualize); #endif From 1a560c7214b29f8d2908875c3410f592480bee7b Mon Sep 17 00:00:00 2001 From: Sebastien Pouliot Date: Mon, 9 Aug 2021 13:44:35 -0400 Subject: [PATCH 3/3] Don't apply optimization on non-AOT builds, e.g. simulators, since some features (like XML serialization) checks for `RuntimeFeature.IsDynamicCodeSupported` and that requires some types to be subclassed thru SRE --- dotnet/targets/Xamarin.Shared.Sdk.targets | 2 +- tests/linker/ios/link all/SealerTest.cs | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/dotnet/targets/Xamarin.Shared.Sdk.targets b/dotnet/targets/Xamarin.Shared.Sdk.targets index 09596f634805..81a584898293 100644 --- a/dotnet/targets/Xamarin.Shared.Sdk.targets +++ b/dotnet/targets/Xamarin.Shared.Sdk.targets @@ -467,7 +467,7 @@ <_ExtraTrimmerArgs>$(_ExtraTrimmerArgs) --disable-opt unusedtypechecks - <_ExtraTrimmerArgs Condition="'$(_BundlerDebug)' != 'true' And '$(MtouchInterpreter)' == '' And '$(_PlatformName)' != 'macOS'">$(_ExtraTrimmerArgs) --enable-opt sealer + <_ExtraTrimmerArgs Condition="'$(_BundlerDebug)' != 'true' And '$(MtouchInterpreter)' == '' And '$(_RunAotCompiler)' == 'true' And '$(_PlatformName)' != 'macOS'">$(_ExtraTrimmerArgs) --enable-opt sealer false diff --git a/tests/linker/ios/link all/SealerTest.cs b/tests/linker/ios/link all/SealerTest.cs index 571e3b669d82..329beb67c3dd 100644 --- a/tests/linker/ios/link all/SealerTest.cs +++ b/tests/linker/ios/link all/SealerTest.cs @@ -36,6 +36,17 @@ public class Subclass : Base, Interface { [Preserve (AllMembers = true)] public class SealerTest { +#if NET + [SetUp] + public void SetUp () + { + // XML serialization mechanism is controlled by RuntimeFeature.IsDynamicCodeSupported + // which will be true for simulator / JIT builds + // so the optimization is disabled unless AOT is used + TestRuntime.AssertDevice (); + } +#endif + [Test] public void Sealed () {