Skip to content

Commit

Permalink
[dotnet][linker] Enable the sealer optimization (#12009)
Browse files Browse the repository at this point in the history
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 c56b893
Fix #9573

Notes

* 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.

* 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
  • Loading branch information
spouliot authored Aug 10, 2021
1 parent 300fc28 commit a20d417
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 9 deletions.
3 changes: 3 additions & 0 deletions dotnet/targets/Xamarin.Shared.Sdk.targets
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,9 @@
-->
<_ExtraTrimmerArgs>$(_ExtraTrimmerArgs) --disable-opt unusedtypechecks</_ExtraTrimmerArgs>

<!-- If a release build and the app is not extensible (no interpreter or JIT/code-loading for macOS) then the sealer optimization can be used -->
<_ExtraTrimmerArgs Condition="'$(_BundlerDebug)' != 'true' And '$(MtouchInterpreter)' == '' And '$(_RunAotCompiler)' == 'true' And '$(_PlatformName)' != 'macOS'">$(_ExtraTrimmerArgs) --enable-opt sealer</_ExtraTrimmerArgs>

<!-- We always want the linker to process debug symbols, even when building in Release mode, because the AOT compiler uses the managed debug symbols to output DWARF debugging symbols -->
<TrimmerRemoveSymbols Condition="'$(TrimmerRemoveSymbols)' == ''">false</TrimmerRemoveSymbols>

Expand Down
20 changes: 11 additions & 9 deletions tests/linker/ios/link all/SealerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,17 @@ public class Subclass : Base, Interface {
[Preserve (AllMembers = true)]
public class SealerTest {

#if !DEBUG && NET
[Ignore ("https://github.com/xamarin/xamarin-macios/issues/9573")]
#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 ()
{
Expand All @@ -62,9 +70,6 @@ public void Sealed ()
#endif
}

#if !DEBUG && NET
[Ignore ("https://github.com/xamarin/xamarin-macios/issues/9573")]
#endif
[Test]
public void Final ()
{
Expand All @@ -81,13 +86,10 @@ 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
}

#if !DEBUG && NET
[Ignore ("https://github.com/xamarin/xamarin-macios/issues/9573")]
#endif
[Test]
public void Virtual ()
{
Expand Down
1 change: 1 addition & 0 deletions tools/linker/MonoTouch.Tuner/SealerSubStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

4 comments on commit a20d417

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

❌ [CI Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

API Diff (from PR only) (no change)
Generator Diff (only version changes)

Packages generated

View packages

Test results

1 tests failed, 228 tests passed.

Failed tests

  • Documentation/All: Failed

Pipeline on Agent XAMBOT-1038.BigSur
[dotnet][linker] Enable the sealer optimization (#12009)

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Tests were not ran (VSTS: device tests tvOS). ⚠️

Results were skipped for this run due to provisioning problems Azure Devops. Please contact the bot administrator.

Pipeline on Agent
[dotnet][linker] Enable the sealer optimization (#12009)

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Tests were not ran (VSTS: device tests iOS). ⚠️

Results were skipped for this run due to provisioning problems Azure Devops. Please contact the bot administrator.

Pipeline on Agent
[dotnet][linker] Enable the sealer optimization (#12009)

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

❌ Tests failed on macOS M1 - Mac Big Sur (11.5) ❌

Tests failed on M1 - Mac Big Sur (11.5).

Failed tests are:

  • xammac_tests

Pipeline on Agent
[dotnet][linker] Enable the sealer optimization (#12009)

Please sign in to comment.