[generator] generator --lang-features=emit-legacy-interface-invokers

[jonpryor]

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 different 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 required. 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()! -- 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 inherited 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_ViewManager = …;
    static readonly JniPeerMembers _members_WindowManager = …;

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

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

This has two advantages:

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

TODO: Performance? How much better is this?
Update tests/invocation-overhead.

Update tests/generator-Tests expected output.
Note: to keep this patch small, only JavaInterop1 output uses the new pattern. XAJavaInterop1 output is unchanged, even though it will use the new output by default.

Added CS0114 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, 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.

[jonpryor]

TODO:

• xamarin-android side test PR
• Update tests/invocation-overhead and verify this is actually faster. (I mean, how can it not? but…)

[jonpryor]

DO NOT MERGE until we're targeting .NET 9. and dotnet/android#8339 is good.

[jonpryor]

Performance Tests?

# JavaTiming_CreateRunnable_Invocations=100, Runnable_Run_Invocations=100
% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop-PerformanceTests.dll --filter "Name~InterfaceInvokerTiming"
…
 ## InterfaceInvokers Timing: instanceIds: 00:00:01.2014814
 ## InterfaceInvokers Timing: peerMembers: 00:00:00.1477179

# JavaTiming_CreateRunnable_Invocations=100, Runnable_Run_Invocations=1000
% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop-PerformanceTests.dll --filter "Name~InterfaceInvokerTiming"
 ## InterfaceInvokers Timing: instanceIds: 00:00:10.2499754
 ## InterfaceInvokers Timing: peerMembers: 00:00:00.1760339

…which makes me think there's something wrong with the current performance tests. Increasing Runnable_Run_Invocations tenfold increases instanceIds time tenfold, but barely touches peerMembers time. Like…how? What is wrong here?

[jpobst]

What is the intention with this flag? Will we use it internally? Is it an escape hatch if there are issues with this implementation? Are we going to publicly expose it for users?

[jonpryor]

Yes, the intention is that if the new idea screws somebody up -- they can't compile, whatever -- then they'll have a way to disable it. As such, yes, we'd need to publicly expose the toggle via a .NET Android MSBuild property.

[jpobst]

It would be nice to add a comment to this property explaining what the format is and what uses it. There are so many *Name properties that I would like to start documenting what each one is for.

[jonpryor]

doc-comment added.