[linker] Add an well known candidate inliner substep along with tests (…

…xamarin#1513)

TL&DR: This is *how* it should be done and tested, it's not complete
(single, simple case) nor the most interesting case ;-)

The trick is to make sure each case is covered by tests so a mono
_bump_ won't give us a BCL that does not conform to what the linker
expect.

What's the impact ?

1. There is the expected reduction of metadata in mscorlib. Since both
   methods don't call other API there's no indirect effect (removal).

--- before	2017-01-15 11:12:44.000000000 -0500
+++ after	2017-01-15 11:12:56.000000000 -0500
@@ -13166,9 +13166,6 @@
 System.Void System.Security.SecurityException::.ctor(System.Runtime.Serialization.SerializationInfo,System.Runtime.Serialization.StreamingContext)
 System.Void System.Security.SecurityException::.ctor(System.String)
 System.Void System.Security.SecurityException::GetObjectData(System.Runtime.Serialization.SerializationInfo,System.Runtime.Serialization.StreamingContext)
-System.Boolean System.Security.SecurityManager::CheckElevatedPermissions()
-System.Security.SecurityManager
-System.Void System.Security.SecurityManager::EnsureElevatedPermissions()
 System.Security.SecurityRulesAttribute
 System.Security.SecurityRuleSet System.Security.SecurityRulesAttribute::m_ruleSet
 System.Void System.Security.SecurityRulesAttribute::.ctor(System.Security.SecurityRuleSet)

2. There is no visible size change (even with #1) in mscorlib.dll due to
   padding (compiler /filealign)

   mscorlib.dll                793,600      793,600            0       0.00 %

3. there's a *very* small reduction of mscorlib.*.aotdata size

   mscorlib.armv7.aotdata      717,264      717,216          -48      -0.01 %
   mscorlib.arm64.aotdata      712,840      712,704         -136      -0.02 %

   AOT data *.aotdata        6,460,064    6,459,880         -184       0.00 %

4. there's no change in executable size - normal as the AOT compiler has
   _likely_ already doing the same optimization (before this commit)

   Executable               29,270,272   29,270,272            0       0.00 %

Full comparison: https://gist.github.com/spouliot/0464c8fa3a92b6486dfd90595d9eb718

tests/linker-ios/link sdk/CanaryTest.cs

@@ -0,0 +1,34 @@
+using System;
+using System.Reflection;
+using System.Text;
+using Foundation;
+using NUnit.Framework;
+
+namespace LinkSdk {
+
+	[TestFixture]
+	[Preserve (AllMembers = true)]
+	public class CanaryTest {
+
+		// if the canary tests fails then something needs to be updated in the linker
+
+		void AssertAbsentType (string typeName)
+		{
+			var t = Type.GetType (typeName);
+			if (t == null)
+				return;
+			var members = t.GetMethods (BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
+			var sb = new StringBuilder (t.FullName);
+			foreach (var m in members)
+				sb.AppendLine ().Append ("* ").Append (m);
+			Assert.Fail (sb.ToString ());
+		}
+
+		[Test]
+		public void Mscorlib ()
+		{
+			// Not critical (on failure) but not optimal - the linker should be able to remove those types entirely
+			AssertAbsentType ("System.Security.SecurityManager, mscorlib");
+		}
+	}
+}

tests/linker-ios/link sdk/LinkExtraDefsTest.cs

@@ -33,8 +33,8 @@ public class LinkExtraDefsTest {
 		[Test]
 		public void Corlib ()
 		{
-			Type t = Type.GetType ("System.Security.HostSecurityManager, mscorlib");
-			Assert.NotNull (t, "System.Security.HostSecurityManager");
+			Type t = Type.GetType ("System.Security.PermissionSet, mscorlib");
+			Assert.NotNull (t, "System.Security.PermissionSet");
 		}
 
 		[Test]

tests/linker-ios/link sdk/extra-linker-defs-tvos.xml

@@ -1,7 +1,7 @@
 <linker>
     <!-- see LinkExtraDefsTest unit tests -->
 	<assembly fullname="mscorlib">
-		<type fullname="System.Security.HostSecurityManager" />
+		<type fullname="System.Security.PermissionSet" />
     </assembly>
 	<assembly fullname="System">
 		<type fullname="System.Net.Mime.ContentType" >

tests/linker-ios/link sdk/extra-linker-defs-watchos.xml

@@ -1,7 +1,7 @@
 <linker>
     <!-- see LinkExtraDefsTest unit tests -->
 	<assembly fullname="mscorlib">
-		<type fullname="System.Security.HostSecurityManager" />
+		<type fullname="System.Security.PermissionSet" />
     </assembly>
 	<assembly fullname="System">
 		<type fullname="System.Net.Mime.ContentType" >

tests/linker-ios/link sdk/extra-linker-defs.xml

@@ -1,7 +1,7 @@
 <linker>
     <!-- see LinkExtraDefsTest unit tests -->
 	<assembly fullname="mscorlib">
-		<type fullname="System.Security.HostSecurityManager" />
+		<type fullname="System.Security.PermissionSet" />
     </assembly>
 	<assembly fullname="System">
 		<type fullname="System.Net.Mime.ContentType" >

tests/linker-ios/link sdk/link sdk.csproj

@@ -171,6 +171,7 @@
     <Compile Include="LocaleTest.cs" />
     <Compile Include="ReflectionTest.cs" />
     <Compile Include="HttpClientHandlerTest.cs" />
+    <Compile Include="CanaryTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\..\BundledResources\BundledResources.csproj">

tests/mtouch/InlinerTest.cs

@@ -0,0 +1,157 @@
+using NUnit.Framework;
+using System;
+using System.IO;
+using System.Linq;
+using System.Collections.Generic;
+using Mono.Cecil;
+using Mono.Cecil.Cil;
+using Xamarin.Tests;
+
+namespace Xamarin.Linker {
+
+	public static partial class Extensions {
+
+		// note: direct check, no inheritance
+		public static bool Is (this TypeReference type, string @namespace, string name)
+		{
+			return ((type != null) && (type.Name == name) && (type.Namespace == @namespace));
+		}
+	}
+
+	public abstract class InlinerTest {
+
+		// note: not every candidate should be handled by the linker 
+		// most of them are _naturally_ eliminated by not being used/marked
+		static bool DisplayCandidates = false;
+
+		protected abstract string Assembly { get; }
+
+		AssemblyDefinition assembly;
+
+		protected virtual AssemblyDefinition AssemblyDefinition {
+			get {
+				if (assembly == null)
+					assembly = AssemblyDefinition.ReadAssembly (Assembly);
+				return assembly;
+			}
+		}
+
+		protected string ListMethods (HashSet<string> h)
+		{
+			string result = Path.GetFileName (Assembly);
+			if (h.Count == 0)
+				return $" {result}: success";
+			return result + "\n" + h.Aggregate ((arg1, arg2) => arg1 + "\n" + arg2);
+		}
+
+		bool IsCandidateForInlining (MethodDefinition m)
+		{
+			// candidates must be methods
+			if (m.IsConstructor)
+				return false;
+			// must have a body (IL)
+			if (!m.HasBody)
+				return false;
+			// must be static or not virtual (can't be overrriden)
+			if (!m.IsStatic && m.IsVirtual)
+				return false;
+			// the body must not have exception handlers or variables
+			var b = m.Body;
+			return !(b.HasExceptionHandlers || b.HasVariables || b.InitLocals);
+		}
+
+		/// <summary>
+		/// We look for candidates, without parameters, that only do `return true;`.
+		/// E.g. Such a static method can be inlined by replacing the `call` with a `ldc.i4.1` instruction
+		/// We must ensure that the list of methods we inline remains unchanged in the BCL we ship.
+		/// </summary>
+		protected void NoParameterReturnOnlyConstant (Code code, HashSet<string> list)
+		{
+			if (DisplayCandidates)
+				Console.WriteLine ($"### NoParameterReturnOnlyConstant {code}: {Path.GetFileName (Assembly)}");
+
+			var ad = AssemblyDefinition.ReadAssembly (Assembly);
+			foreach (var t in ad.MainModule.Types) {
+				if (!t.HasMethods)
+					continue;
+				foreach (var m in t.Methods) {
+					if (!IsCandidateForInlining (m))
+						continue;
+					if (m.HasParameters)
+						continue;
+					var b = m.Body;
+					if (b.Instructions.Count != 2)
+						continue;
+					var ins = b.Instructions [0];
+					if (code == ins.OpCode.Code) {
+						var s = m.ToString ();
+						list.Remove (s);
+						if (DisplayCandidates)
+							Console.WriteLine ($"* `{s}`");
+					}
+				}
+			}
+		}
+
+		/// <summary>
+		/// We look for candidates, without parameters and return value, that does nothing (only `ret`).
+		/// E.g. Such a static method can be inlined by replacing the `call` with a nop` instruction.
+		/// We must ensure that the list of methods we inline remains unchanged in the BCL we ship.
+		/// </summary>
+		protected void NoParameterNoReturnNoCode (HashSet<string> list)
+		{
+			if (DisplayCandidates)
+				Console.WriteLine ($"### ReturnOnly: {Path.GetFileName (Assembly)}");
+
+			foreach (var t in AssemblyDefinition.MainModule.Types) {
+				if (!t.HasMethods)
+					continue;
+				foreach (var m in t.Methods) {
+					if (!IsCandidateForInlining (m))
+						continue;
+					if (m.HasParameters)
+						continue;
+					if (!m.ReturnType.Is ("System", "Void"))
+						continue;
+					var b = m.Body;
+					if (b.Instructions.Count == 1) {
+						var s = m.ToString ();
+						list.Remove (s);
+						if (DisplayCandidates)
+							Console.WriteLine ($"* `{s}`");
+					}
+				}
+			}
+		}
+	}
+
+	[TestFixture]
+	public class MscorlibInlinerTest : InlinerTest {
+
+		protected override string Assembly {
+			get { return Path.Combine (Configuration.MonoTouchRootDirectory, "lib", "mono", "Xamarin.iOS", "mscorlib.dll"); }
+		}
+
+		[Test]
+		public void True ()
+		{
+			// list MUST be kept in sync with InlinerSubStep.cs
+			var h = new HashSet<string> {
+				"System.Boolean System.Security.SecurityManager::CheckElevatedPermissions()",
+			};
+			NoParameterReturnOnlyConstant (Code.Ldc_I4_1, h);
+			Assert.That (h, Is.Empty, ListMethods (h));
+		}
+
+		[Test]
+		public void Nop ()
+		{
+			// this list MUST be kept in sync with InlinerSubStep.cs
+			var h = new HashSet<string> {
+				"System.Void System.Security.SecurityManager::EnsureElevatedPermissions()",
+			};
+			NoParameterNoReturnNoCode (h);
+			Assert.That (h, Is.Empty, ListMethods (h));
+		}
+	}
+}

tests/mtouch/mtouch.csproj

@@ -48,9 +48,16 @@
     <Compile Include="TimingTests.cs" />
     <Compile Include="MLaunchTool.cs" />
     <Compile Include="Cache.cs" />
+    <Compile Include="InlinerTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <None Include="packages.config" />
   </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\..\external\mono\external\cecil\Mono.Cecil.csproj">
+      <Project>{D68133BD-1E63-496E-9EDE-4FBDBF77B486}</Project>
+      <Name>Mono.Cecil</Name>
+    </ProjectReference>
+  </ItemGroup>
   <Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
 </Project>

tools/linker/MonoTouch.Tuner/InlinerSubStep.cs

@@ -0,0 +1,60 @@
+// Copyright 2016-2017 Xamarin Inc.
+
+using System;
+using Mono.Cecil;
+using Mono.Cecil.Cil;
+using Mono.Linker;
+using Mono.Tuner;
+
+namespace Xamarin.Linker.Steps {
+
+	// This inlining is done, almost exclusively, as a metadata reduction step that
+	// occurs before linking so some code is not marked (and shipped in final apps).
+	//
+	// In many case the AOT'ed native code won't be affected (same size) but the
+	// *.aotdata files will be smaller. In some cases the AOT compiler does not 
+	// inline some _simple_ cases (cross assemblies) so it can reduce the native 
+	// executable size too (but this is not the step main goal)
+	public class InlinerSubStep : ExceptionalSubStep {
+
+		protected override string Name { get; } = "Inliner";
+		protected override int ErrorCode { get; } = 2090;
+
+		public override SubStepTargets Targets {
+			get {
+				return SubStepTargets.Type | SubStepTargets.Method;
+			}
+		}
+
+		public override bool IsActiveFor (AssemblyDefinition assembly)
+		{
+			return Annotations.GetAction (assembly) == AssemblyAction.Link;
+		}
+
+		protected override void Process (MethodDefinition method)
+		{
+			if (!method.HasBody)
+				return;
+
+			foreach (var il in method.Body.Instructions) {
+				if (il.OpCode.Code == Code.Call) {
+					var mr = il.Operand as MethodReference;
+					if (mr == null)
+						continue;
+					// this removes type System.Security.SecurityManager (unless referenced by user code)
+					if (!mr.HasParameters && mr.DeclaringType.Is ("System.Security", "SecurityManager")) {
+						switch (mr.Name) {
+						case "EnsureElevatedPermissions":
+							il.OpCode = OpCodes.Nop;
+							break;
+						case "CheckElevatedPermissions":
+							// always positive (no security manager)
+							il.OpCode = OpCodes.Ldc_I4_1;
+							break;
+						}
+					}
+				}
+			}
+		}
+	}
+}

tools/mtouch/Makefile

@@ -54,6 +54,7 @@ LINKER_SOURCES = \
 	$(MONO_TUNER)/Mono.Tuner/RemoveResources.cs			\
 	$(MONO_TUNER)/Mono.Tuner/RemoveSecurity.cs			\
 	$(LINKER_DIR)/MonoTouch.Tuner/Extensions.cs			\
+	$(LINKER_DIR)/MonoTouch.Tuner/InlinerSubStep.cs		\
 	$(LINKER_DIR)/MonoTouch.Tuner/ListExportedSymbols.cs	\
 	$(LINKER_DIR)/MonoTouch.Tuner/MetadataReducerSubStep.cs	\
 	$(LINKER_DIR)/MonoTouch.Tuner/MonoTouchMarkStep.cs		\

tools/mtouch/Tuning.cs

@@ -114,14 +114,15 @@ static SubStepDispatcher GetSubSteps (LinkerOptions options)
 			sub.Add (new CoreRemoveSecurity ());
 			sub.Add (new OptimizeGeneratedCodeSubStep (options));
 			sub.Add (new RemoveUserResourcesSubStep (options));
-			// OptimizeGeneratedCodeSubStep and RemoveNativeCodeSubStep needs [GeneratedCode] so it must occurs before RemoveAttributes
+			// OptimizeGeneratedCodeSubStep and RemoveUserResourcesSubStep needs [GeneratedCode] so it must occurs before RemoveAttributes
 			sub.Add (new RemoveAttributes ());
 			// http://bugzilla.xamarin.com/show_bug.cgi?id=1408
 			if (options.LinkAway)
 				sub.Add (new RemoveCode (options));
 			sub.Add (new MarkNSObjects ());
 			sub.Add (new PreserveSoapHttpClients ());
 			sub.Add (new CoreHttpMessageHandler (options));
+			sub.Add (new InlinerSubStep ());
 			return sub;
 		}
 

tools/mtouch/error.cs

@@ -218,6 +218,7 @@ namespace Xamarin.Bundler {
 	//					MT206x	Sealer failed processing `...`.
 	//					MT207x	Metadata Reducer failed processing `...`.
 	//					MT208x	MarkNSObjects failed processing `...`.
+	//					MT209x	Inliner failed processing `...`.
 	// MT3xxx	AOT
 	//			MT30xx	AOT (general) errors
 	//					MT3001	Could not AOT the assembly '{0}'

tools/mtouch/mtouch.csproj

@@ -316,6 +316,9 @@
     <Compile Include="..\linker\RemoveUserResourcesSubStep.cs">
       <Link>Xamarin.Linker\RemoveUserResourcesSubStep.cs</Link>
     </Compile>
+    <Compile Include="..\linker\MonoTouch.Tuner\InlinerSubStep.cs">
+      <Link>MonoTouch.Tuner\InlinerSubStep.cs</Link>
+    </Compile>
   </ItemGroup>
   <ItemGroup>
     <Reference Include="System.Core" />