[linker] remove unneeded string.Format() calls (#4260)

I was using the Mono profiler for our build, and I noticed:

	Allocation summary
	    Bytes      Count  Average Type name
	180765632    2142803       84 System.String
	 53218472 bytes from:
	        MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.AssemblyDefinition)
	        MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethodsUnconditional (Mono.Cecil.AssemblyDefinition)
	        MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.TypeDefinition)
	        MonoDroid.Tuner.FixAbstractMethodsStep:HaveSameSignature (Mono.Cecil.TypeReference,Mono.Cecil.MethodDefinition,Mono.Cecil.MethodDefinition)
	        (wrapper managed-to-native) string:FastAllocateString (int)

~53MB of string from this one method is a lot!

I found that one of these `string.Format()` calls are running for
almost every method on every interface in every assembly:

	? (string.Format ("{0}.{1}", iface.FullName, iMethod.Name) != tMethod.Name)
	: (string.Format ("{0}.{1}.{2}", iMethod.DeclaringType.DeclaringType, iface.Name, iMethod.Name) != tMethod.Name))

Even the simplest non-matching cases would call `string.Format()`:

	iMethod.Name == "Foo"
	tMethod.Name == "Bar"

In all of these cases...

  * Class implements interface implicitly, or
  * Class implements interface explicitly, or
  * Class implements abstract class

We found that either the `IsInOverrides()` check should work or
comparing `iMethod.Name` and `tMethod.Name` should work.  We don't
seem to need the `string.Format()` calls at all?

~~ Results ~~

An initial build of the Xamarin.Forms integration project on
Windows/.NET framework saves ~30ms:

  * Before:

        796 ms  LinkAssembliesNoShrink                     1 calls

  * After:

        767 ms  LinkAssembliesNoShrink                     1 calls

The same project on macOS/Mono (slightly slower machine, too)
saves ~316ms:

  * Before:

        1341 ms  LinkAssembliesNoShrink                     1 calls

  * After:

        1025 ms  LinkAssembliesNoShrink                     1 calls

String allocations are way better, too:

  * Before:

        Allocation summary
            Bytes      Count  Average Type name
        180765632    2142803       84 System.String

  * After:

        Allocation summary
            Bytes      Count  Average Type name
        127736832    1652070       77 System.String

Shows 53,028,800 bytes saved.

  * Before:

        53218472 bytes from:
            MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.AssemblyDefinition)
            MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethodsUnconditional (Mono.Cecil.AssemblyDefinition)
            MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.TypeDefinition)
            MonoDroid.Tuner.FixAbstractMethodsStep:HaveSameSignature (Mono.Cecil.TypeReference,Mono.Cecil.MethodDefinition,Mono.Cecil.MethodDefinition)

  * After:

         1933624 bytes from:
            MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethodsUnconditional (Mono.Cecil.AssemblyDefinition)
            MonoDroid.Tuner.FixAbstractMethodsStep:FixAbstractMethods (Mono.Cecil.TypeDefinition)
            MonoDroid.Tuner.FixAbstractMethodsStep:HaveSameSignature (Mono.Cecil.TypeReference,Mono.Cecil.MethodDefinition,Mono.Cecil.MethodDefinition)
            MonoDroid.Tuner.FixAbstractMethodsStep:HaveSameMethodName (Mono.Cecil.TypeReference,Mono.Cecil.MethodDefinition,Mono.Cecil.MethodDefinition)

Shows 51,284,848 bytes saved.

I don't know about the discrepancy between these two sets of numbers.
But I think this is roughly ~50MB less string allocations.

Author: jonathanpeppers

Documentation/release-notes/4260.md

@@ -0,0 +1,24 @@
+### Linker Behavior Change
+
+Xamarin.Android has to consolidate changes to the Android APIs and how
+they affect C# types. If you consider an example such as:
+
+* API 28 has an `IFoo` interface in `Mono.Android.dll` for `v9.0`.
+* C# code implements `IFoo` in class `Bar`.
+* API 29 adds another method to `IFoo` in `Mono.Android.dll` for
+  `v10.0`.
+* Apps built targeting API 30 / `v10.0` will fix up `Bar` to implement
+  the new method, but throw `Java.Lang.AbstractMethodError` if it was
+  called.
+
+If the linker didn't make this change, you would instead hit a crash
+when trying to create an instance of type `Bar` such as:
+
+    System.TypeLoadException: VTable setup of type 'Bar' failed.
+
+Xamarin.Android has some performance improvements to the linker in
+this area. Existing apps *should* not be affected.
+
+Submit a [bug report][bug] if you find an issue in this area.
+
+[bug]: https://github.com/xamarin/xamarin-android/wiki/Submitting-Bugs,-Feature-Requests,-and-Pull-Requests

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

@@ -158,7 +158,7 @@ bool HaveSameSignature (TypeReference iface, MethodDefinition iMethod, MethodDef
 			if (IsInOverrides (iMethod, tMethod))
 				return true;
 
-			if (iMethod.Name != tMethod.Name && (iMethod.DeclaringType == null || (iMethod.DeclaringType.DeclaringType == null ? (string.Format ("{0}.{1}", iface.FullName, iMethod.Name) != tMethod.Name) : (string.Format ("{0}.{1}.{2}", iMethod.DeclaringType.DeclaringType, iface.Name, iMethod.Name) != tMethod.Name))))
+			if (iMethod.Name != tMethod.Name)
 				return false;
 
 			if (!CompareTypes (iMethod.MethodReturnType.ReturnType, tMethod.MethodReturnType.ReturnType))

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs

@@ -70,6 +70,69 @@ static void CreateAbstractIfaceImplementation (string assemblyPath, AssemblyDefi
 			}
 		}
 
+		[Test]
+		public void FixAbstractMethodsStep_Explicit ()
+		{
+			var path = Path.Combine (Path.GetFullPath (XABuildPaths.TestOutputDirectory), "temp", TestName);
+			var step = new FixAbstractMethodsStep ();
+			var pipeline = new Pipeline ();
+
+			Directory.CreateDirectory (path);
+
+			using (var context = new LinkContext (pipeline)) {
+
+				context.Resolver.AddSearchDirectory (path);
+
+				var myAssemblyPath = Path.Combine (path, "MyAssembly.dll");
+
+				using (var android = CreateFauxMonoAndroidAssembly ()) {
+					android.Write (Path.Combine (path, "Mono.Android.dll"));
+					CreateExplicitInterface (myAssemblyPath, android);
+				}
+
+				using (var assm = context.Resolve (myAssemblyPath)) {
+					step.Process (context);
+
+					var impl = assm.MainModule.GetType ("MyNamespace.MyClass");
+					Assert.AreEqual (2, impl.Methods.Count, "MyClass should contain 2 methods");
+					var method = impl.Methods.FirstOrDefault (m => m.Name == "MyNamespace.IMyInterface.MyMethod");
+					Assert.IsNotNull (method, "MyNamespace.IMyInterface.MyMethod should exist");
+					method = impl.Methods.FirstOrDefault (m => m.Name == "MyMissingMethod");
+					Assert.IsNotNull (method, "MyMissingMethod should exist");
+				}
+			}
+
+			Directory.Delete (path, true);
+		}
+
+		static void CreateExplicitInterface (string assemblyPath, AssemblyDefinition android)
+		{
+			using (var assm = AssemblyDefinition.CreateAssembly (new AssemblyNameDefinition ("NestedIFaceTest", new Version ()), "NestedIFaceTest", ModuleKind.Dll)) {
+				var void_type = assm.MainModule.ImportReference (typeof (void));
+
+				// Create interface
+				var iface = new TypeDefinition ("MyNamespace", "IMyInterface", TypeAttributes.Interface);
+
+				var iface_method = new MethodDefinition ("MyMethod", MethodAttributes.Abstract, void_type);
+				iface.Methods.Add (iface_method);
+				iface.Methods.Add (new MethodDefinition ("MyMissingMethod", MethodAttributes.Abstract, void_type));
+
+				assm.MainModule.Types.Add (iface);
+
+				// Create implementing class
+				var jlo = assm.MainModule.Import (android.MainModule.GetType ("Java.Lang.Object"));
+				var impl = new TypeDefinition ("MyNamespace", "MyClass", TypeAttributes.Public, jlo);
+				impl.Interfaces.Add (new InterfaceImplementation (iface));
+
+				var explicit_method = new MethodDefinition ("MyNamespace.IMyInterface.MyMethod", MethodAttributes.Abstract, void_type);
+				explicit_method.Overrides.Add (new MethodReference (iface_method.Name, void_type, iface));
+				impl.Methods.Add (explicit_method);
+
+				assm.MainModule.Types.Add (impl);
+				assm.Write (assemblyPath);
+			}
+		}
+
 		static AssemblyDefinition CreateFauxMonoAndroidAssembly ()
 		{
 			var assm = AssemblyDefinition.CreateAssembly (new AssemblyNameDefinition ("Mono.Android", new Version ()), "DimTest", ModuleKind.Dll);