Optimize calls to BlockLiteral.SetupBlock to inject the block signatu…

…re. (dotnet#3391)

* [linker] Optimize calls to BlockLiteral.SetupBlock to inject the block signature.

Optimize calls to BlockLiteral.SetupBlock[Unsafe] to calculate the block
signature at build time, and inject it into the call site.

This makes block invocations 10-15x faster (I've added tests that asserts at
least an 8x increase).

It's also required in order to be able to remove the dynamic registrar code in
the future (since calculating the block signature at runtime requires the
dynamic registrar).

* [mtouch/mmp] Add support for reporting errors/warnings that point to the code line causing the error/warning.

Add support for reporting errors/warnings that point to the code line causing
the error/warning by adding ErrorHelper overloads that take the exact
instruction to report (previously we defaulted to the first line/instruction
in a method).

* [tests] Add support for asserting filename/linenumber in warning messages.

* Make all methods that manually create BlockLiterals optimizable.

* [tests] Create a BaseOptimizeGeneratedCodeTest test that's included in both XI's and XM's link all test.

* [tests] Add link all test (for both XI and XM) to test the BlockLiteral.SetupBlock optimization.

* [tests] Add mtouch/mmp tests for the BlockLiteral.SetupBlock optimization.

* [tests][linker] Make the base test class abstract, so tests in the base class aren't executed twice.

* [tests][linker] Don't execute linkall-only tests in linksdk.

The optimization tests only apply when the test assembly is linked, and that
only happens in linkall, so exclude those tests in linksdk.

* [tests][mmptest] Update test according to mmp changes.

Fixes these test failures:

    1) Failed : Xamarin.MMP.Tests.MMPTests.MM0132("inline-runtime-arch")
    The warning 'MM0132: Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.' was not found in the output:
    	Message #1 did not match:
    		actual:   'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
    		expected: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
    	Message #2 did not match:
    		actual:   'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
    		expected: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'

    2) Failed : Xamarin.MMP.Tests.MMPTests.MM0132("foo")
    The warning 'MM0132: Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.' was not found in the output:
    	Message #1 did not match:
    		actual:   'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
    		expected: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
    	Message #2 did not match:
    		actual:   'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
    		expected: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'

* [tests][linker] Fix typo.

Fixes this test failure:

    1) SetupBlock_CustomDelegate (Linker.Shared.BaseOptimizeGeneratedCodeTest.SetupBlock_CustomDelegate)
         Counter
      Expected: 1
      But was:  2

* [registrar] Minor adjustment to error message to match previous (and better) behavior.

Fixes this test failure:

    1) Failed : Xamarin.Registrar.GenericType_WithInvalidParameterTypes
    The error 'MT4136: The registrar cannot marshal the parameter type 'System.Collections.Generic.List`1<U>' of the parameter 'arg' in the method 'Open`1.Bar(System.Collections.Generic.List`1<U>)'' was not found in the output:
    	Message #1 did not match:
    		actual:   'The registrar cannot marshal the parameter type 'System.Collections.Generic.List`1<Foundation.NSObject>' of the parameter 'arg' in the method 'Open`1.Bar(System.Collections.Generic.List`1<U>)''
    		expected: 'The registrar cannot marshal the parameter type 'System.Collections.Generic.List`1<U>' of the parameter 'arg' in the method 'Open`1.Bar(System.Collections.Generic.List`1<U>)''

* [docs] mmp shows MM errors/warnings.

* [docs] Improve according to reviews.

* [tests] Fix merge failure causing test duplication.

docs/website/mmp-errors.md

@@ -225,6 +225,17 @@ Mixed-mode assemblies can not be processed by the linker.
 
 See https://msdn.microsoft.com/en-us/library/x0w2664k.aspx for more information on mixed-mode assemblies.
 
+### <a name="MM2106"/>MM2106: Could not optimize the call to BlockLiteral.SetupBlock[Unsafe] in * at offset * because *.
+
+The linker reports this warning when it can't optimize a call to BlockLiteral.SetupBlock or Block.SetupBlockUnsafe.
+
+The message will point to the method that calls BlockLiteral.SetupBlock[Unsafe], and
+it may also give clues as to why the call couldn't be optimized.
+
+Please file an [issue](https://github.com/xamarin/xamarin-macios/issues/new)
+along with a complete build log so that we can investigate what went wrong and
+possibly enable more scenarios in the future.
+
 # MM3xxx: AOT
 
 ## MM30xx: AOT (general) errors
@@ -252,6 +263,27 @@ See https://msdn.microsoft.com/en-us/library/x0w2664k.aspx for more information
 
 ### <a name="MM4134">MM4134: Your application is using the '{0}' framework, which isn't included in the MacOS SDK you're using to build your app (this framework was introduced in OSX {2}, while you're building with the MacOS {1} SDK.) This configuration is not supported with the static registrar (pass --registrar:dynamic as an additional mmp argument in your project's Mac Build option to select). Alternatively select a newer SDK in your app's Mac Build options.
 
+### <a name="MM4173"/>MM4173: The registrar can't compute the block signature for the delegate of type {delegate-type} in the method {method} because *.
+
+This is a warning indicating that the registrar couldn't inject the block
+signature of the specified method into the generated registrar code, because
+the registrar couldn't compute it.
+
+This means that the block signature has to be computed at runtime, which is
+somewhat slower.
+
+There are currently two possible reasons for this warning:
+
+1. The type of the managed delegate is either a `System.Delegate` or
+   `System.MulticastDelegate`. These types don't represent a specific signature,
+   which means the registrar can't compute the corresponding native signature
+   either. In this case the fix is to use a specific delegate type for the
+   block (alternatively the warning can be ignored by adding `--nowarn:4173`
+   as an additional mmp argument in the project's Mac Build options).
+2. The registrar can't find the `Invoke` method of the delegate. This
+   shouldn't happen, so please file an [issue](https://github.com/xamarin/xamarin-macios/issues/new)
+   with a test project so that we can fix it.
+
 # MM5xxx: GCC and toolchain
 
 ## MM51xx: compilation

docs/website/mtouch-errors.md

@@ -1300,6 +1300,17 @@ See https://msdn.microsoft.com/en-us/library/x0w2664k.aspx for more information
 
 The `[BindingImpl]` attribute on the mentioned member is invalid. The expected format is `[BindingImpl (BindingImplOptions.ValueA | BindingImplOptions.ValueB)]`.
 
+### <a name="MT2106"/>MT2106: Could not optimize the call to BlockLiteral.SetupBlock[Unsafe] in * at offset * because *.
+
+The linker reports this warning when it can't optimize a call to BlockLiteral.SetupBlock or Block.SetupBlockUnsafe.
+
+The message will point to the method that calls BlockLiteral.SetupBlock[Unsafe], and
+it may also give clues as to why the call couldn't be optimized.
+
+Please file an [issue](https://github.com/xamarin/xamarin-macios/issues/new)
+along with a complete build log so that we can investigate what went wrong and
+possibly enable more scenarios in the future.
+
 # MT3xxx: AOT error messages
 
 <!--
@@ -1650,6 +1661,27 @@ with a test case and we'll evaluate it.
 [1]: https://bugzilla.xamarin.com/enter_bug.cgi?product=iOS
 [2]: https://bugzilla.xamarin.com/enter_bug.cgi?product=iOS&component=General&bug_severity=enhancement
 
+### <a name="MT4173"/>MT4173: The registrar can't compute the block signature for the delegate of type {delegate-type} in the method {method} because *.
+
+This is a warning indicating that the registrar couldn't inject the block
+signature of the specified method into the generated registrar code, because
+the registrar couldn't compute it.
+
+This means that the block signature has to be computed at runtime, which is
+somewhat slower.
+
+There are currently two possible reasons for this warning:
+
+1. The type of the managed delegate is either a `System.Delegate` or
+   `System.MulticastDelegate`. These types don't represent a specific signature,
+   which means the registrar can't compute the corresponding native signature
+   either. In this case the fix is to use a specific delegate type for the
+   block (alternatively the warning can be ignored by adding `--nowarn:4173`
+   as an additional mtouch argument in the project's iOS Build options).
+2. The registrar can't find the `Invoke` method of the delegate. This
+   shouldn't happen, so please file an [issue](https://github.com/xamarin/xamarin-macios/issues/new)
+   with a test project so that we can fix it.
+
 # MT5xxx: GCC and toolchain error messages
 
 ### MT51xx: Compilation

docs/website/optimizations.md

@@ -304,3 +304,47 @@ methods with the `[BindingImpl (BindingImplOptions.Optimizable)]` attribute.
 It is always enabled by default (when the linker is enabled).
 
 The default behavior can be overridden by passing `--optimize=[+|-]dead-code-elimination` to mtouch/mmp.
+
+Optimize calls to BlockLiteral.SetupBlock
+-----------------------------------------
+
+The Xamarin.iOS/Mac runtime needs to know the block signature when creating an
+Objective-C block for a managed delegate. This might be a fairly expensive
+operation. This optimization will calculate the block signature at build time,
+and modify the IL to call a `SetupBlock` method that takes the signature as an
+argument instead. Doing this avoids the need for calculating the signature at
+runtime.
+
+Benchmarks show that this speeds up calling a block by a factor of 10 to 15.
+
+It will transform the following [code](https://github.com/xamarin/xamarin-macios/blob/018f7153441d9d7e0f58e2046f39eeb46f1ff480/src/UIKit/UIAccessibility.cs#L198-L211):
+
+```csharp
+public static void RequestGuidedAccessSession (bool enable, Action<bool> completionHandler)
+{
+	// ...
+	block_handler.SetupBlock (callback, completionHandler);
+	// ...
+}
+```
+
+into:
+
+```csharp
+public static void RequestGuidedAccessSession (bool enable, Action<bool> completionHandler)
+{
+	// ...
+	block_handler.SetupBlockImpl (callback, completionHandler, true, "v@?B");
+	// ...
+}
+```
+
+This optimization requires the linker to be enabled, and is only applied to
+methods with the `[BindingImpl (BindingImplOptions.Optimizable)]` attribute.
+
+It is enabled by default when using the static registrar (in Xamarin.iOS the
+static registrar is enabled by default for device builds, and in Xamarin.Mac
+the static registrar is enabled by default for release builds).
+
+The default behavior can be overridden by passing `--optimize=[+|-]blockliteral-setupblock` to mtouch/mmp.
+

runtime/delegates.t4

@@ -46,7 +46,8 @@
 
 		new XDelegate ("id", "IntPtr", "xamarin_create_delegate_proxy",
 			"MonoObject *", "IntPtr", "method",
-			"MonoObject *", "IntPtr", "block"
+			"MonoObject *", "IntPtr", "block",
+			"const char *", "IntPtr", "signature"
 		) { WrappedManagedFunction = "CreateDelegateProxy" },
 
 		new XDelegate ("void", "void", "xamarin_register_entry_assembly",

runtime/runtime.m

@@ -2077,10 +2077,10 @@ -(void) xamarinSetGCHandle: (int) gc_handle;
 }
 
 id
-xamarin_get_block_for_delegate (MonoMethod *method, MonoObject *delegate, guint32 *exception_gchandle)
+xamarin_get_block_for_delegate (MonoMethod *method, MonoObject *delegate, const char *signature, guint32 *exception_gchandle)
 {
 	// COOP: accesses managed memory: unsafe mode.
-	return delegates.create_delegate_proxy ((MonoObject *) mono_method_get_object (mono_domain_get (), method, NULL), delegate, exception_gchandle);
+	return delegates.create_delegate_proxy ((MonoObject *) mono_method_get_object (mono_domain_get (), method, NULL), delegate, signature, exception_gchandle);
 }
 
 void

runtime/trampolines.m

@@ -71,7 +71,7 @@
 		case _C_PTR: {
 			MonoClass *klass = mono_class_from_mono_type (mtype);
 			if (mono_class_is_delegate (klass)) {
-				return xamarin_get_block_for_delegate (method, retval, exception_gchandle);
+				return xamarin_get_block_for_delegate (method, retval, NULL, exception_gchandle);
 			} else {
 				return *(void **) mono_object_unbox (retval);
 			}

runtime/xamarin/runtime.h

@@ -142,7 +142,7 @@ MonoType *		xamarin_get_parameter_type (MonoMethod *managed_method, int index);
 MonoObject *	xamarin_get_nsobject_with_type_for_ptr (id self, bool owns, MonoType* type, guint32 *exception_gchandle);
 MonoObject *	xamarin_get_nsobject_with_type_for_ptr_created (id self, bool owns, MonoType *type, int32_t *created, guint32 *exception_gchandle);
 int *			xamarin_get_delegate_for_block_parameter (MonoMethod *method, int par, void *nativeBlock, guint32 *exception_gchandle);
-id              xamarin_get_block_for_delegate (MonoMethod *method, MonoObject *delegate, guint32 *exception_gchandle);
+id              xamarin_get_block_for_delegate (MonoMethod *method, MonoObject *delegate, const char *signature /* NULL allowed, but requires the dynamic registrar at runtime to compute */, guint32 *exception_gchandle);
 id				xamarin_get_nsobject_handle (MonoObject *obj);
 void			xamarin_set_nsobject_handle (MonoObject *obj, id handle);
 uint8_t         xamarin_get_nsobject_flags (MonoObject *obj);

src/AddressBook/ABAddressBook.cs

@@ -207,6 +207,7 @@ public static ABAuthorizationStatus GetAuthorizationStatus ()
 		extern unsafe static void ABAddressBookRequestAccessWithCompletion (IntPtr addressbook, void * completion);
 
 		[iOS (6,0)]
+		[BindingImpl (BindingImplOptions.Optimizable)]
 		public void RequestAccess (Action<bool,NSError> onCompleted)
 		{
 			if (onCompleted == null)

src/AudioToolbox/SystemSound.cs

@@ -192,6 +192,7 @@ static unsafe void TrampolineAction (IntPtr blockPtr)
 		}
 
 		[iOS (9,0)][Mac (10,11)]
+		[BindingImpl (BindingImplOptions.Optimizable)]
 		public void PlayAlertSound (Action onCompletion)
 		{
 			if (onCompletion == null)
@@ -224,6 +225,7 @@ public Task PlayAlertSoundAsync ()
 		}
 
 		[iOS (9,0)][Mac (10,11)]
+		[BindingImpl (BindingImplOptions.Optimizable)]
 		public void PlaySystemSound (Action onCompletion)
 		{
 			if (onCompletion == null)

src/CoreFoundation/DispatchBlock.cs

@@ -21,6 +21,7 @@ internal class DispatchBlock {
 		// You must invoke ->CleanupBlock after you have transferred ownership to
 		// the unmanaged code to release the resources allocated on the managed side
 		//
+		[BindingImpl (BindingImplOptions.Optimizable)]
 		public static unsafe void Invoke (Action codeToRun, Action<IntPtr> invoker)
 		{
 			BlockLiteral *block_ptr;

src/CoreText/CTLine.cs

@@ -251,6 +251,7 @@ static unsafe void TrampolineEnumerate (IntPtr blockPtr, double offset, nint cha
  		}
 
 		[iOS (9,0)][Mac (10,11)]
+		[BindingImpl (BindingImplOptions.Optimizable)]
 		public void EnumerateCaretOffsets (CaretEdgeEnumerator enumerator)
 		{
 			if (enumerator == null)

src/Metal/MTLDevice.cs

@@ -64,6 +64,7 @@ public static IMTLDevice [] GetAllDevices ()
 		unsafe static extern IntPtr MTLCopyAllDevicesWithObserver (ref IntPtr observer, void* handler);
 
 		[Mac (10, 13, onlyOn64: true), NoiOS, NoWatch, NoTV]
+		[BindingImpl (BindingImplOptions.Optimizable)]
 		public static IMTLDevice [] GetAllDevices (ref NSObject observer, MTLDeviceNotificationHandler handler)
 		{
 			if (observer == null)

src/ObjCRuntime/Blocks.cs

@@ -83,29 +83,18 @@ public unsafe struct BlockLiteral {
 		[DllImport ("__Internal")]
 		static extern IntPtr xamarin_get_block_descriptor ();
 
-		unsafe void SetupBlock (Delegate trampoline, Delegate userDelegate, bool safe)
+		void SetupBlock (Delegate trampoline, Delegate userDelegate, bool safe)
 		{
-			isa = block_class;
-			invoke = Marshal.GetFunctionPointerForDelegate (trampoline);
-			object delegates;
-			if (safe) {
-				delegates = new Tuple<Delegate, Delegate> (trampoline, userDelegate);
-			} else {
-				delegates = userDelegate;
-			}
-			local_handle = (IntPtr) GCHandle.Alloc (delegates);
-			global_handle = IntPtr.Zero;
-			flags = BlockFlags.BLOCK_HAS_COPY_DISPOSE | BlockFlags.BLOCK_HAS_SIGNATURE;
-
-			/* FIXME: support stret blocks */
-
 			// We need to get the signature of the target method, so that we can compute
 			// the ObjC signature correctly (the generated method that's actually
 			// invoked by native code does not have enough type information to compute
 			// the correct signature).
 			// This attribute might not exist for third-party libraries created
 			// with earlier versions of Xamarin.iOS, so make sure to cope with
 			// the attribute not being available.
+
+			// This logic is mirrored in CoreOptimizeGeneratedCode.ProcessSetupBlock and must be
+			// updated if anything changes here.
 			var userDelegateType = trampoline.GetType ().GetCustomAttribute<UserDelegateTypeAttribute> ()?.UserDelegateType;
 			bool blockSignature;
 			MethodInfo userMethod;
@@ -117,12 +106,34 @@ unsafe void SetupBlock (Delegate trampoline, Delegate userDelegate, bool safe)
 				blockSignature = false;
 			}
 
+			var signature = Runtime.ComputeSignature (userMethod, blockSignature);
+			SetupBlockImpl (trampoline, userDelegate, safe, signature);
+		}
+
+		// This method is not to be called manually by user code.
+		// This is enforced by making it private. If the SetupBlock optimization is enabled,
+		// the linker will make it public so that it's callable from optimized user code.
+		unsafe void SetupBlockImpl (Delegate trampoline, Delegate userDelegate, bool safe, string signature)
+		{
+			isa = block_class;
+			invoke = Marshal.GetFunctionPointerForDelegate (trampoline);
+			object delegates;
+			if (safe) {
+				delegates = new Tuple<Delegate, Delegate> (trampoline, userDelegate);
+			} else {
+				delegates = userDelegate;
+			}
+			local_handle = (IntPtr) GCHandle.Alloc (delegates);
+			global_handle = IntPtr.Zero;
+			flags = BlockFlags.BLOCK_HAS_COPY_DISPOSE | BlockFlags.BLOCK_HAS_SIGNATURE;
+
+			/* FIXME: support stret blocks */
+
 			// we allocate one big block of memory, the first part is the BlockDescriptor, 
 			// the second part is the signature string (no need to allocate a second time
 			// for the signature if we can avoid it). One descriptor is allocated for every 
 			// Block; this is potentially something the static registrar can fix, since it
 			// should know every possible trampoline signature.
-			var signature = Runtime.ComputeSignature (userMethod, blockSignature);
 			var bytes = System.Text.Encoding.UTF8.GetBytes (signature);
 			var desclen = sizeof (XamarinBlockDescriptor) + bytes.Length + 1 /* null character */;
 			var descptr = Marshal.AllocHGlobal (desclen);
@@ -228,7 +239,7 @@ public static bool IsManagedBlock (IntPtr block)
 			return descriptor->copy_helper == ((BlockDescriptor *) literal->block_descriptor)->copy_helper;
 		}
 
-		internal static IntPtr GetBlockForDelegate (MethodInfo minfo, object @delegate)
+		internal static IntPtr GetBlockForDelegate (MethodInfo minfo, object @delegate, string signature)
 		{
 			if (@delegate == null)
 				return IntPtr.Zero;
@@ -263,7 +274,11 @@ internal static IntPtr GetBlockForDelegate (MethodInfo minfo, object @delegate)
 			// call _Block_copy, which will create a heap-allocated block
 			// with the proper reference count.
 			BlockLiteral block = new BlockLiteral ();
-			block.SetupBlock ((Delegate) handlerDelegate, (Delegate) @delegate);
+			if (signature == null) {
+				block.SetupBlock ((Delegate) handlerDelegate, (Delegate) @delegate);
+			} else {
+				block.SetupBlockImpl ((Delegate) handlerDelegate, (Delegate) @delegate, true, signature);
+			}
 			var rv = _Block_copy (ref block);
 			block.CleanupBlock ();
 			return rv;