[generator] generator --lang-features=emit-legacy-interface-invokers (#1145)

Fixes: #910

Context: bc5bcf4
Context: #858

Consider the Java `java.lang.Runnable` interface:

	package java.lang;
	public interface Runnable {
	    void run ();
	}

This is bound as:

	package Java.Lang;
	public interface IRunnable : IJavaPeerable {
	    void Run ();
	}

with some slight differences depending on whether we're dealing with
.NET Android (`generator --codegen-target=xajavainterop1`) or
`src/Java.Base` (`generator --codegen-target=javainterop1`).

Now, assume a Java API + corresponding binding which returns a
`Runnable` instance:

	package example;
	public class Whatever {
	    public static Runnable createRunnable();
	}

You can invoke `IRunnable.Run()` on the return value:

	IRunnable r = Whatever.CreateRunnable();
	r.Run();

but how does that work?

This works via an "interface Invoker", which is a class emitted by
`generator` which implements the interface and invokes the interface
methods through JNI:

	internal partial class IRunnableInvoker : Java.Lang.Object, IRunnable {
	    public void Run() => …
	}

Once Upon A Time™, the interface invoker implementation mirrored that
of classes: a static `IntPtr` field held the `jmethodID` value, which
would be looked up on first-use and cached for subsequent invocations:

	partial class IRunnableInvoker {
	    static IntPtr id_run;
	    public unsafe void Run() {
	        if (id_run == IntPtr.Zero)
	            id_run = JNIEnv.GetMethodID (class_ref, "run", "()V");
	        JNIEnv.CallVoidMethod (Handle, id_run, …);
	    }
	}

This approach works until you have interface inheritance and methods
which come from inherited interfaces:

	package android.view;
	public /* partial */ interface ViewManager {
	    void addView(View view, ViewGroup.LayoutParams params);
	}
	public /* partial */ interface WindowManager extends ViewManager {
	    void removeViewImmediate(View view);
	}

This would be bound as:

	namespace Android.Views;
	public partial interface IViewManager : IJavaPeerable {
	    void AddView (View view, ViewGroup.LayoutParams @params);
	}
	public partial IWindowManager : IViewManager {
	    void RemoveViewImmediate (View view);
	}
	internal partial class IWindowManagerInvoker : Java.Lang.Object, IWindowManager {
	    static IntPtr id_addView;
	    public void AddView(View view, ViewGroup.LayoutParams @params)
	    {
	        if (id_addView == IntPtr.Zero)
	            id_run = JNIEnv.GetMethodID (class_ref, "addView", "…");
	        JNIEnv.CallVoidMethod (Handle, id_addView, …);
	    }
	}

Unfortunately, *invoking* `IViewManager.AddView()` through an
`IWindowManagerInvoker` would crash!

	D/dalvikvm( 6645): GetMethodID: method not found: Landroid/view/WindowManager;.addView:(Landroid/view/View;Landroid/view/ViewGroup$LayoutParams;)V
	I/MonoDroid( 6645): UNHANDLED EXCEPTION: Java.Lang.NoSuchMethodError: Exception of type 'Java.Lang.NoSuchMethodError' was thrown.
	I/MonoDroid( 6645): at Android.Runtime.JNIEnv.GetMethodID (intptr,string,string)
	I/MonoDroid( 6645): at Android.Views.IWindowManagerInvoker.AddView (Android.Views.View,Android.Views.ViewGroup/LayoutParams)
	I/MonoDroid( 6645): at Mono.Samples.Hello.HelloActivity.OnCreate (Android.OS.Bundle)
	I/MonoDroid( 6645): at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_ (intptr,intptr,intptr)
	I/MonoDroid( 6645): at (wrapper dynamic-method) object.ecadbe0b-9124-445e-a498-f351075f6c89 (intptr,intptr,intptr)

Interfaces are not classes, and this is one of the places that this
is most apparent.  Because of this crash, we had to use *instance*
`jmethodID` caches:

	internal partial class IWindowManagerInvoker : Java.Lang.Object, IWindowManager {
	    IntPtr id_addView;
	    public void AddView(View view, ViewGroup.LayoutParams @params)
	    {
	        if (id_addView == IntPtr.Zero)
	            id_run = JNIEnv.GetMethodID (class_ref, "addView", "…");
	        JNIEnv.CallVoidMethod (Handle, id_addView, …);
	    }
	}

Pro: no more crash!

Con: *every different instance* of `IWindowManagerInvoker` needs to
separately lookup whatever methods are invoked.  There is *some*
caching, so repeated calls to `AddView()` on the same instance will
hit the cache, but if you obtain a different `IWindowManager`
instance, `jmethodID` values will need to be looked up again.

This was "fine", until #858 enters the picture:
interface invokers were full of Android-isms --
`Android.Runtime.JNIEnv.GetMethodID()`! `JNIEnv.CallVoidMethod()`! --
and thus ***not*** APIs that @jonpryor wished to expose within
desktop Java.Base bindings.

Enter `generator --lang-features=emit-legacy-interface-invokers`:
when *not* specified, interface invokers will now use
`JniPeerMembers` for method lookup and invocation, allowing
`jmethodID` values to be cached *across* instances.  In order to
prevent the runtime crash, an interface may have *multiple*
`JniPeerMembers` values, one per implemented interface, which is used
to invoke methods from that interface.

`IWindowManagerInvoker` now becomes:

	internal partial class IWindowManagerInvoker : Java.Lang.Object, IWindowManager {
	    static readonly JniPeerMembers _members_android_view_ViewManager    = …;
	    static readonly JniPeerMembers _members_android_view_WindowManager  = …;

	    public void AddView(View view, ViewGroup.LayoutParams @params)
	    {
	        const string __id = "addView.…";
	        _members_android_view_ViewManager.InstanceMethods.InvokeAbstractVoidMethod (__id, this, …);
	    }

	    public void RemoveViewImmediate(View view)
	    {
	        const string __id = "removeViewImmediate.…";
	        _members_android_view_WindowManager.InstanceMethods.InvokeAbstractVoidMethod (__id, this, …);
	    }
	}

This has two advantages:

 1. More caching!
 2. Desktop `Java.Base` binding can now have interface invokers.

Update `tests/generator-Tests` expected output.
Note: to keep this patch smaller, JavaInterop1 output uses the
new pattern, and only *some* XAJavaInterop1 tests use the new
pattern.

Added [CS0114][0] to `$(NoWarn)` in `Java.Base.csproj` to ignore
warnings such as:

	…/src/Java.Base/obj/Debug-net7.0/mcw/Java.Lang.ICharSequence.cs(195,25): warning CS0114:
	'ICharSequenceInvoker.ToString()' hides inherited member 'Object.ToString()'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

[Ignoring CS0114 is also done in `Mono.Android.dll` as well][1], so
this is not a new or unique requirement.

Update `Java.Interop.dll` so that
`JniRuntime.JniValueManager.GetActivationConstructor()` now knows
about and looks for `*Invoker` types, then uses the activation
constructor from the `*Invoker` type when the source type is an
abstract `class` or `interface`.

Update `tests/Java.Base-Tests` to test for implicit `*Invoker` lookup
and invocation support.


~~ Property Setters ~~

While testing on dotnet/android#8339, we hit this error
(among others, to be addressed later):

	src/Mono.Android/obj/Debug/net8.0/android-34/mcw/Android.Views.IWindowInsetsController.cs(304,41): error CS0103: The name 'behavior' does not exist in the current context

This was caused because of code such as:

	public partial interface IWindowInsetsController {
	    public unsafe int SystemBarsBehavior {
	        get {
	            const string __id = "getSystemBarsBehavior.()I";
	            try {
	                var __rm = _members_IWindowInsetsController.InstanceMethods.InvokeAbstractInt32Method (__id, this, null);
	                return __rm;
	            } finally {
	            }
	        }
	        set {
	            const string __id = "setSystemBarsBehavior.(I)V";
	            try {
	                JniArgumentValue* __args = stackalloc JniArgumentValue [1];
	                __args [0] = new JniArgumentValue (behavior);
	                _members_IWindowInsetsController.InstanceMethods.InvokeAbstractVoidMethod (__id, this, __args);
	            } finally {
	            }
	        }
	    }
	}

This happened because when emitting the property setter, we need
to update the `set*` method's parameter name to be `value` so that
the normal property setter body is emitted properly.

Update `InterfaceInvokerProperty.cs` so that the parameter name
is set to `value`.


~~ Performance ~~

What does this do for performance?

Add a new `InterfaceInvokerTiming` test fixture to
`Java.Interop-PerformanceTests.dll`, which:

 1. "Reimplements" the "legacy" and "JniPeerMembers" Invoker
    strategies
 2. For each Invoker strategy:
     a. Invokes a Java method which returns a `java.lang.Runnable`
        instance
     b. Invokes `Runnable.run()` on the instance returned by (2.a)
        …100 times.
     c. Repeat (2.a) and (2.b) 100 times.

The result is that using `JniPeerMembers` is *much* faster:

	% dotnet build tests/Java.Interop-PerformanceTests/*.csproj && \
		dotnet test --logger "console;verbosity=detailed"  bin/TestDebug-net7.0/Java.Interop-PerformanceTests.dll --filter "Name~InterfaceInvokerTiming"
	…
	 Passed InterfaceInvokers [1 s]
	 Standard Output Messages:
	## InterfaceInvokers Timing: instanceIds: 00:00:01.1095502
	## InterfaceInvokers Timing: peerMembers: 00:00:00.1400427

Using `JniPeerMembers` takes ~1/8th the time as using `jmethodID`s.

TODO: something is *probably* wrong with my test -- reviews welcome!
-- as when I increase the (2.b) iteration count, the `peerMembers`
time is largely unchanged (~0.14s), while the `instanceIds` time
increases linearly.

*Something* is wrong there.  I'm not sure what.  (Or *nothing* is
wrong, and instance `jmethodID` are just *that* bad.)


[0]: https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0114
[1]: https://github.com/xamarin/xamarin-android/blob/d5c4ec09f7658428a10bbe49c8a7a3eb2f71cb86/src/Mono.Android/Mono.Android.csproj#L12C7-L12C7

Author: jonpryor

build-tools/automation/templates/core-tests.yaml

@@ -132,7 +132,7 @@ steps:
   inputs:
     command: test
     testRunTitle: Java.Interop-Performance ($(DotNetTargetFramework) - ${{ parameters.platformName }})
-    arguments: bin/Test$(Build.Configuration)$(NetCoreTargetFrameworkPathSuffix)/Java.Interop-PerformanceTests.dll
+    arguments: --logger "console;verbosity=detailed" bin/Test$(Build.Configuration)$(NetCoreTargetFrameworkPathSuffix)/Java.Interop-PerformanceTests.dll
   continueOnError: true
   retryCountOnTaskFailure: 1
 

src/Java.Base/Java.Base.csproj

@@ -4,7 +4,7 @@
     <TargetFramework>$(DotNetTargetFramework)</TargetFramework>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <Nullable>enable</Nullable>
-    <NoWarn>$(NoWarn);8764</NoWarn>
+    <NoWarn>$(NoWarn);8764;0114</NoWarn>
   </PropertyGroup>
 
   <Import Project="..\..\TargetFrameworkDependentValues.props" />

src/Java.Base/Java.Lang/ICharSequence.cs

@@ -1,5 +1,10 @@
+using System.Collections;
+
 namespace Java.Lang {
 
+	partial class ICharSequenceInvoker : IEnumerable {
+	}
+
 	public static partial class ICharSequenceExtensions {
 
 		public static ICharSequence[]? ToCharSequenceArray (this string?[]? values)

src/Java.Base/Transforms/Metadata.xml

@@ -1,7 +1,10 @@
 <metadata>
-  <!-- For now, just bind java.lang.* -->
-  <remove-node path="//api/package[not(starts-with(@name, 'java.lang')
-      or starts-with(@name, 'java.io')
+  <!-- For now, just bind a few packages  -->
+  <remove-node path="//api/package[
+      not(
+        starts-with(@name, 'java.lang')
+        or starts-with(@name, 'java.io')
+        or starts-with(@name, 'java.util.function')
       )]" />
 
   <!-- Type / Namespace conflicts -->
@@ -54,6 +57,9 @@
       ]/method[@name='write']"
       name="explicitInterface">IDataOutput</attr>
 
+  <!-- CS0108 but for *static* members; TODO: how do we fix? -->
+  <remove-node path="/api/package[@name='java.util.function']/interface[@name='UnaryOperator']/method[@name='identity' and count(parameter)=0]" />
+
   <!-- AbstractStringBuilder is package-private; fixity fix -->
   <remove-node path="//api/package[@name='java.lang']/class[@name='AbstractStringBuilder']" />
 

src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs

@@ -335,14 +335,33 @@ static Type GetPeerType (Type type)
 
 			static ConstructorInfo? GetActivationConstructor (Type type)
 			{
-				return
-					(from c in type.GetConstructors (BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
-					 let p = c.GetParameters ()
-					 where p.Length == 2 && p [0].ParameterType == ByRefJniObjectReference && p [1].ParameterType == typeof (JniObjectReferenceOptions)
-					 select c)
-				.FirstOrDefault ();
+				if (type.IsAbstract || type.IsInterface) {
+					type = GetInvokerType (type) ?? type;
+				}
+				foreach (var c in type.GetConstructors (BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)) {
+					var p = c.GetParameters ();
+					if (p.Length == 2 && p [0].ParameterType == ByRefJniObjectReference && p [1].ParameterType == typeof (JniObjectReferenceOptions))
+						return c;
+				}
+				return null;
 			}
 
+			static Type? GetInvokerType (Type type)
+			{
+				const string suffix = "Invoker";
+				Type[] arguments = type.GetGenericArguments ();
+				if (arguments.Length == 0)
+					return type.Assembly.GetType (type + suffix);
+				Type definition = type.GetGenericTypeDefinition ();
+				int bt = definition.FullName!.IndexOf ("`", StringComparison.Ordinal);
+				if (bt == -1)
+					throw new NotSupportedException ("Generic type doesn't follow generic type naming convention! " + type.FullName);
+				Type? suffixDefinition = definition.Assembly.GetType (
+						definition.FullName.Substring (0, bt) + suffix + definition.FullName.Substring (bt));
+				if (suffixDefinition == null)
+					return null;
+				return suffixDefinition.MakeGenericType (arguments);
+			}
 
 			public object? CreateValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType = null)
 			{

tests/Java.Base-Tests/Java.Base/JavaToManagedTests.cs

@@ -18,6 +18,18 @@ public void InterfaceMethod ()
 			Assert.IsTrue (invoked);
 			r.Dispose ();
 		}
+
+		[Test]
+		public void InterfaceInvokerMethod ()
+		{
+			int value   = 0;
+			using var c = new MyIntConsumer (v => value = v);
+			using var r = JavaInvoker.CreateRunnable (c);
+			r?.Run ();
+			Assert.AreEqual (0, value);
+			r?.Run ();
+			Assert.AreEqual (1, value);
+		}
 	}
 
 	class JavaInvoker : JavaObject {
@@ -31,6 +43,14 @@ public static unsafe void Run (Java.Lang.IRunnable r)
 			args [0] = new JniArgumentValue (r);
 			_members.StaticMethods.InvokeVoidMethod ("run.(Ljava/lang/Runnable;)V", args);
 		}
+
+		public static unsafe Java.Lang.IRunnable? CreateRunnable (Java.Util.Function.IIntConsumer c)
+		{
+			JniArgumentValue* args = stackalloc JniArgumentValue [1];
+			args [0] = new JniArgumentValue (c);
+			var _rm  = _members.StaticMethods.InvokeObjectMethod ("createRunnable.(Ljava/util/function/IntConsumer;)Ljava/lang/Runnable;", args);
+			return Java.Interop.JniEnvironment.Runtime.ValueManager.GetValue<Java.Lang.IRunnable> (ref _rm, JniObjectReferenceOptions.CopyAndDispose);
+		}
 	}
 
 	[JniTypeSignature ("example/MyRunnable")]
@@ -48,4 +68,20 @@ public void Run ()
 			action ();
 		}
 	}
+
+	[JniTypeSignature ("example/MyIntConsumer")]
+	class MyIntConsumer : Java.Lang.Object, Java.Util.Function.IIntConsumer {
+
+		Action<int> action;
+
+		public MyIntConsumer (Action<int> action)
+		{
+			this.action = action;
+		}
+
+		public void Accept (int value)
+		{
+			action (value);
+		}
+	}
 }

tests/Java.Base-Tests/java/com/microsoft/java_base_tests/Invoker.java

@@ -1,8 +1,19 @@
 package com.microsoft.java_base_tests;
 
-public class Invoker {
+import java.util.function.IntConsumer;
+
+public final class Invoker {
 
 	public static void run(Runnable r) {
 		r.run();
 	}
+
+	public static Runnable createRunnable(final IntConsumer consumer) {
+		return new Runnable() {
+			int value;
+			public void run() {
+				consumer.accept(value++);
+			}
+		};
+	}
 }

tests/Java.Interop-PerformanceTests/Java.Interop/JavaTiming.cs

@@ -276,6 +276,11 @@ public unsafe JniObjectReference Timing_ToString_JniPeerMembers ()
 			const string id = toString_name + "." + toString_sig;
 			return _members.InstanceMethods.InvokeVirtualObjectMethod (id, this, null);
 		}
+
+		public static unsafe JniObjectReference CreateRunnable ()
+		{
+			return _members.StaticMethods.InvokeObjectMethod ("CreateRunnable.()Ljava/lang/Runnable;", null);
+		}
 	}
 
 	[JniTypeSignature (JniTypeName)]

tests/Java.Interop-PerformanceTests/Java.Interop/TimingTests.cs

@@ -776,6 +776,41 @@ public void GenericMarshalingOverhead_Int32ArrayArrayArray ()
 			total.Stop ();
 			Console.WriteLine ("## {0} Timing: {1}", nameof (GenericMarshalingOverhead_Int32ArrayArrayArray), total.Elapsed);
 		}
+
+	}
+
+	[TestFixture]
+	public class InterfaceInvokerTiming : Java.InteropTests.JavaVMFixture {
+
+		[Test]
+		public void InterfaceInvokers ()
+		{
+			const int JavaTiming_CreateRunnable_Invocations = 100;
+			const int Runnable_Run_Invocations              = 100;
+
+			var instanceIds   = Stopwatch.StartNew ();
+			for (int i = 0; i < JavaTiming_CreateRunnable_Invocations; ++i) {
+				var c = JavaTiming.CreateRunnable ();
+				IMyRunnable r = new LegacyRunnableInvoker (ref c, JniObjectReferenceOptions.CopyAndDispose);
+				for (int j = 0; j < Runnable_Run_Invocations; ++j) {
+					r.Run ();
+				}
+				r.Dispose ();
+			}
+			instanceIds.Stop ();
+			var peerMembers   = Stopwatch.StartNew ();
+			for (int i = 0; i < JavaTiming_CreateRunnable_Invocations; ++i) {
+				var c = JavaTiming.CreateRunnable ();
+				IMyRunnable r = new JniPeerMembersRunnableInvoker (ref c, JniObjectReferenceOptions.CopyAndDispose);
+				for (int j = 0; j < Runnable_Run_Invocations; ++j) {
+					r.Run ();
+				}
+				r.Dispose ();
+			}
+			peerMembers.Stop ();
+			Console.WriteLine ("## {0} Timing: instanceIds: {1}", nameof (InterfaceInvokers), instanceIds.Elapsed);
+			Console.WriteLine ("## {0} Timing: peerMembers: {1}", nameof (InterfaceInvokers), peerMembers.Elapsed);
+		}
 	}
 
 	class ManagedTiming {
@@ -893,5 +928,60 @@ public override object GetValue ()
 			return null;
 		}
 	}
+
+	interface IMyRunnable : IJavaPeerable {
+		void Run();
+	}
+
+	class LegacyRunnableInvoker : JavaObject, IMyRunnable {
+		static readonly JniPeerMembers _members = new JniPeerMembers ("java/lang/Runnable", typeof (LegacyRunnableInvoker));
+		JniObjectReference class_ref;
+
+		public LegacyRunnableInvoker (ref JniObjectReference reference, JniObjectReferenceOptions options)
+			: base (ref reference, options)
+		{
+			var r = JniEnvironment.Types.GetObjectClass (PeerReference);
+			class_ref = r.NewGlobalRef ();
+			JniObjectReference.Dispose (ref r);
+		}
+
+		public override JniPeerMembers JniPeerMembers {
+			get { return _members; }
+		}
+
+		protected override void Dispose (bool disposing)
+		{
+			JniObjectReference.Dispose (ref class_ref);
+			base.Dispose (disposing);
+		}
+
+		JniMethodInfo id_run;
+
+		public unsafe void Run ()
+		{
+			if (id_run == null) {
+				id_run = JniEnvironment.InstanceMethods.GetMethodID (class_ref, "run", "()V");
+			}
+			JniEnvironment.InstanceMethods.CallObjectMethod (PeerReference, id_run);
+		}
+	}
+
+	class JniPeerMembersRunnableInvoker : JavaObject, IMyRunnable {
+		public JniPeerMembersRunnableInvoker (ref JniObjectReference reference, JniObjectReferenceOptions options)
+			: base (ref reference, options)
+		{
+		}
+
+		static readonly JniPeerMembers _members_IRunnable = new JniPeerMembers ("java/lang/Runnable", typeof (JniPeerMembersRunnableInvoker));
+
+		public unsafe void Run ()
+		{
+			const string __id = "run.()V";
+			try {
+				_members_IRunnable.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null);
+			} finally {
+			}
+		}
+	}
 }
 

tests/Java.Interop-PerformanceTests/java/com/xamarin/interop/performance/JavaTiming.java

@@ -77,5 +77,14 @@ public static void StaticVoidMethod2IArgs (int obj1, int obj2)
 	public static void StaticVoidMethod3IArgs (int obj1, int obj2, int obj3)
 	{
 	}
+
+	public static Runnable CreateRunnable ()
+	{
+		return new Runnable () {
+			public void run ()
+			{
+			}
+		};
+	}
 }