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

Fixes: #910
Context: bc5bcf4
Context: https://github.com/xamarin/java.interop/issues/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 xamarin/java.interop#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>
 
  <ImportProject="..\..\TargetFrameworkDependentValues.props" />

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

@@ -1,5 +1,10 @@
+usingSystem.Collections;
+
 namespaceJava.Lang{
 
+partialclassICharSequenceInvoker:IEnumerable{
+}
+
 publicstaticpartialclassICharSequenceExtensions{
 
 publicstaticICharSequence[]?ToCharSequenceArray(thisstring?[]?values)

src/Java.Base/Transforms/Metadata.xml

@@ -1,7 +1,10 @@
 <metadata>
-<!-- For now, just bind java.lang.* -->
- <remove-nodepath="//api/package[not(starts-with(@name, 'java.lang')
- or starts-with(@name, 'java.io')
+<!-- For now, just bind a few packages -->
+ <remove-nodepath="//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-nodepath="/api/package[@name='java.util.function']/interface[@name='UnaryOperator']/method[@name='identity' and count(parameter)=0]" />
+
 <!-- AbstractStringBuilder is package-private; fixity fix -->
  <remove-nodepath="//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)
 
 staticConstructorInfo?GetActivationConstructor(Typetype)
 {
-return
-(fromcintype.GetConstructors(BindingFlags.Public|BindingFlags.NonPublic|BindingFlags.Instance)
-letp=c.GetParameters()
-wherep.Length==2&&p[0].ParameterType==ByRefJniObjectReference&&p[1].ParameterType==typeof(JniObjectReferenceOptions)
-selectc)
-.FirstOrDefault();
+if(type.IsAbstract||type.IsInterface){
+type=GetInvokerType(type)??type;
+}
+foreach(varcintype.GetConstructors(BindingFlags.Public|BindingFlags.NonPublic|BindingFlags.Instance)){
+varp=c.GetParameters();
+if(p.Length==2&&p[0].ParameterType==ByRefJniObjectReference&&p[1].ParameterType==typeof(JniObjectReferenceOptions))
+returnc;
+}
+returnnull;
 }
 
+staticType?GetInvokerType(Typetype)
+{
+conststringsuffix="Invoker";
+Type[]arguments=type.GetGenericArguments();
+if(arguments.Length==0)
+returntype.Assembly.GetType(type+suffix);
+Typedefinition=type.GetGenericTypeDefinition();
+intbt=definition.FullName!.IndexOf("`",StringComparison.Ordinal);
+if(bt==-1)
+thrownewNotSupportedException("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)
+returnnull;
+returnsuffixDefinition.MakeGenericType(arguments);
+}
 
 publicobject?CreateValue(refJniObjectReferencereference,JniObjectReferenceOptionsoptions,Type?targetType=null)
 {

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

@@ -18,6 +18,18 @@ public void InterfaceMethod ()
 Assert.IsTrue(invoked);
 r.Dispose();
 }
+
+[Test]
+publicvoidInterfaceInvokerMethod()
+{
+intvalue=0;
+usingvarc=newMyIntConsumer(v =>value=v);
+usingvarr=JavaInvoker.CreateRunnable(c);
+r?.Run();
+Assert.AreEqual(0,value);
+r?.Run();
+Assert.AreEqual(1,value);
+}
 }
 
 classJavaInvoker:JavaObject{
@@ -31,6 +43,14 @@ public static unsafe void Run (Java.Lang.IRunnable r)
 args[0]=newJniArgumentValue(r);
 _members.StaticMethods.InvokeVoidMethod("run.(Ljava/lang/Runnable;)V",args);
 }
+
+publicstaticunsafeJava.Lang.IRunnable?CreateRunnable(Java.Util.Function.IIntConsumerc)
+{
+JniArgumentValue*args=stackallocJniArgumentValue[1];
+args[0]=newJniArgumentValue(c);
+var_rm=_members.StaticMethods.InvokeObjectMethod("createRunnable.(Ljava/util/function/IntConsumer;)Ljava/lang/Runnable;",args);
+returnJava.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")]
+classMyIntConsumer:Java.Lang.Object,Java.Util.Function.IIntConsumer{
+
+Action<int>action;
+
+publicMyIntConsumer(Action<int>action)
+{
+this.action=action;
+}
+
+publicvoidAccept(intvalue)
+{
+action(value);
+}
+}
 }

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

@@ -1,8 +1,19 @@
 packagecom.microsoft.java_base_tests;
 
-publicclassInvoker {
+importjava.util.function.IntConsumer;
+
+publicfinalclassInvoker {
 
 publicstaticvoidrun(Runnabler) {
 r.run();
 	}
+
+publicstaticRunnablecreateRunnable(finalIntConsumerconsumer) {
+returnnewRunnable() {
+intvalue;
+publicvoidrun() {
+consumer.accept(value++);
+ }
+ };
+	}
 }

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

@@ -276,6 +276,11 @@ public unsafe JniObjectReference Timing_ToString_JniPeerMembers ()
 conststringid=toString_name+"."+toString_sig;
 return_members.InstanceMethods.InvokeVirtualObjectMethod(id,this,null);
 }
+
+publicstaticunsafeJniObjectReferenceCreateRunnable()
+{
+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]
+publicclassInterfaceInvokerTiming:Java.InteropTests.JavaVMFixture{
+
+[Test]
+publicvoidInterfaceInvokers()
+{
+constintJavaTiming_CreateRunnable_Invocations=100;
+constintRunnable_Run_Invocations=100;
+
+varinstanceIds=Stopwatch.StartNew();
+for(inti=0;i<JavaTiming_CreateRunnable_Invocations;++i){
+varc=JavaTiming.CreateRunnable();
+IMyRunnabler=newLegacyRunnableInvoker(refc,JniObjectReferenceOptions.CopyAndDispose);
+for(intj=0;j<Runnable_Run_Invocations;++j){
+r.Run();
+}
+r.Dispose();
+}
+instanceIds.Stop();
+varpeerMembers=Stopwatch.StartNew();
+for(inti=0;i<JavaTiming_CreateRunnable_Invocations;++i){
+varc=JavaTiming.CreateRunnable();
+IMyRunnabler=newJniPeerMembersRunnableInvoker(refc,JniObjectReferenceOptions.CopyAndDispose);
+for(intj=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);
+}
 }
 
 classManagedTiming{
@@ -893,5 +928,60 @@ public override object GetValue ()
 returnnull;
 }
 }
+
+interfaceIMyRunnable:IJavaPeerable{
+voidRun();
+}
+
+classLegacyRunnableInvoker:JavaObject,IMyRunnable{
+staticreadonlyJniPeerMembers_members=newJniPeerMembers("java/lang/Runnable",typeof(LegacyRunnableInvoker));
+JniObjectReferenceclass_ref;
+
+publicLegacyRunnableInvoker(refJniObjectReferencereference,JniObjectReferenceOptionsoptions)
+:base(refreference,options)
+{
+varr=JniEnvironment.Types.GetObjectClass(PeerReference);
+class_ref=r.NewGlobalRef();
+JniObjectReference.Dispose(refr);
+}
+
+publicoverrideJniPeerMembersJniPeerMembers{
+get{return_members;}
+}
+
+protectedoverridevoidDispose(booldisposing)
+{
+JniObjectReference.Dispose(refclass_ref);
+base.Dispose(disposing);
+}
+
+JniMethodInfoid_run;
+
+publicunsafevoidRun()
+{
+if(id_run==null){
+id_run=JniEnvironment.InstanceMethods.GetMethodID(class_ref,"run","()V");
+}
+JniEnvironment.InstanceMethods.CallObjectMethod(PeerReference,id_run);
+}
+}
+
+classJniPeerMembersRunnableInvoker:JavaObject,IMyRunnable{
+publicJniPeerMembersRunnableInvoker(refJniObjectReferencereference,JniObjectReferenceOptionsoptions)
+:base(refreference,options)
+{
+}
+
+staticreadonlyJniPeerMembers_members_IRunnable=newJniPeerMembers("java/lang/Runnable",typeof(JniPeerMembersRunnableInvoker));
+
+publicunsafevoidRun()
+{
+conststring__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)
 publicstaticvoidStaticVoidMethod3IArgs (intobj1, intobj2, intobj3)
 	{
 	}
+
+publicstaticRunnableCreateRunnable ()
+	{
+returnnewRunnable () {
+publicvoidrun ()
+ {
+ }
+ };
+	}
 }