Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[generator] Use custom delegates instead of Func/Action. #632

Merged
merged 4 commits into from
Apr 28, 2020
Merged

[generator] Use custom delegates instead of Func/Action. #632

merged 4 commits into from
Apr 28, 2020

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Apr 21, 2020 •
edited
Loading

Fixes #631.

Currently we do not support Java methods that contain more than 14 parameters, because we rely on Func<> and Action<> which are capped at 16 arguments (we need 2 for internal usage). This PR instead generates custom delegates as needed for the method signatures required. This allows us to create delegates for any number of parameters.

Also commits a generator.slnf solution filter file which makes it easier to work with generator in VS by only loading needed projects from Java.Interop.sln.

Spec

Implementation approach: within generator, instead of hardcoding Action and Func use, we instead need a "2 pass" design.

We define a pattern for defining delegate type names:

  • Type name prefix: _JniMarshal_PP
  • Parameter types, using JNI encoding, e.g. Z is boolean, I is int, etc. Exception: Reference types, normally encoded as L…;, are instead just L. Kotlin unsigned types are encoded as lowercase versions of the standard JNI encoding, e.g. i is uint.
  • Another _.
  • Return type, encoded as with parameters. A void return type is V.

Consider, Java.Lang.Object.Equals():

static Delegate GetEquals_Ljava_lang_Object_Handler ()
{
	if (cb_equals_Ljava_lang_Object_ == null)
		cb_equals_Ljava_lang_Object_ = JNINativeWrapper.CreateDelegate ((Func<IntPtr, IntPtr, IntPtr, bool>) n_Equals_Ljava_lang_Object_);
	return cb_equals_Ljava_lang_Object_;
}

The delegate type on line 4 would instead be _JniMarshal_PPL_Z, resulting in:

static Delegate GetEquals_Ljava_lang_Object_Handler ()
{
	if (cb_equals_Ljava_lang_Object_ == null)
		cb_equals_Ljava_lang_Object_ = JNINativeWrapper.CreateDelegate ((_JniMarshal_PPL_Z) n_Equals_Ljava_lang_Object_);
	return cb_equals_Ljava_lang_Object_;
}

That's the "first" pass.

The second pass is that we need to collect all the types that we're now mentioning in ^^, and then emit them as global internal types:

delegate bool _JniMarshal_PPL_Z (IntPtr jnienv, IntPtr klass, IntPtr a);

These delegates are generated in the __NamespaceMapping__.cs file.

jonpryor
jonpryor reviewed Apr 22, 2020
View reviewed changes
tools/generator/Java.Interop.Tools.Generator.ObjectModel/NamespaceMapping.cs Outdated
foreach (var p in mappings)
sw.WriteLine ("[assembly:global::Android.Runtime.NamespaceMapping (Java = \"{0}\", Managed=\"{1}\")]",
p.Key, p.Value);

sw.WriteLine ();
sw.WriteLine ("internal class JniDelegates {");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why emit these into a JniDelegates type instead of leaving the delegates as "global"?

Copy link
Contributor Author

@jpobst jpobst Apr 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize delegates could exist outside of a type. TIL!

@jpobst jpobst force-pushed the custom-delegates branch 2 times, most recently from 6ae9653 to 2ae4953 Compare April 23, 2020 14:21
jonpryor
jonpryor reviewed Apr 23, 2020
View reviewed changes
tools/generator/CodeGenerationOptions.cs Outdated

sb.Append ("_");

sb.Append (method.IsVoid ? "V" : GetJniTypeCode (method.RetVal.Symbol));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure the .IsVoid check is required; shouldn't method.RetVal already be an ISymbol instance for void, in which symbol.JniName==V?

@jpobst jpobst force-pushed the custom-delegates branch 2 times, most recently from beb7a88 to 0166274 Compare April 28, 2020 18:25
@jpobst jpobst marked this pull request as ready for review April 28, 2020 18:59
@jpobst
Copy link
Contributor Author

jpobst commented Apr 28, 2020

Release notes:

- [Java.Interop GitHub PR 632](https://github.com/xamarin/java.interop/pull/632):
  Android Bindings projects now support binding Java methods with unlimited
  parameters instead of the previous limit of 14 parameters.

@jonpryor jonpryor merged commit 56955d9 into master Apr 28, 2020
@jonpryor jonpryor deleted the custom-delegates branch April 28, 2020 23:33
@jonpryor jonpryor mentioned this pull request Apr 30, 2020
Closed
jonpryor pushed a commit that referenced this pull request May 6, 2020
@jpobst @jonpryor
[generator] Use custom delegates instead of Func/Action. (#632)
8e5310b 
Fixes: #631

Context: dotnet/runtime#32963
Context: https://github.com/dotnet/csharplang/blob/master/proposals/function-pointers.md

*Of `Delegate`s and JNI Callbacks…*

~~ Background ~~

In order for Java code to invoke Managed Code such as C#, several
things must happen:

 1. There must be a Java class which declares `native` methods.
 2. The Java class' `native` methods must be [*resolvable*][0]

Java `native` method resolution can be done by [C function name][1]
*or* by using [`JNIEnv::RegisterNatives()`][2]:

	// C++
	struct JNINativeMethod {
	    const char *name;
	    const char *signature;
	    const void *fnPtr;
	};

	/* partial */ struct JNIEnv {
	    jint RegisterNatives(jclass clazz, const JNINativeMethod *methods, jint nMethods);
	};

`JNINativeMethods::fnPtr` is a pointer to a *C callable function*
that accepts [JNI Native Method Arguments][3].

Java.Interop doesn't currently support resolution via C function name,
and instead binds the `JNINativeMethod` struct as
`JniNativeMethodRegistration`, and `JNIEnv::RegisterNatives()` as
`Java.Interop.JniEnvironment.Types.RegisterNatives()`:

	// C#
	public partial struct JniNativeMethodRegistration {
	    public  string    Name;
	    public  string    Signature;
	    public  Delegate  Marshaler;
	}
	public partial class JniEnvironment {
	    public partial class Types {
	        public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods);
	    }
	}

Through the glory that is [Platform Invoke Delegate Marshaling][4]
and/or [`Marshal.GetFunctionPointerForDelegate()`][5], managed code
can provide a `Delegate` instance in
`JniNativeMethodRegistration.Marshaler` and have JNI invoke that
delegate when the corresponding Java `native` method is invoked.

`tools/generator` is responsible for emitting this glue code, e.g.
in order to support registering overrides of
[`java.lang.Object.equals()`][6]:

	// C# emitted by `tools/generator`:
	namespace Java.Lang {
	  partial class Object {
	    static Delegate cb_equals_Ljava_lang_Object_;
	    static Delegate GetEquals_Ljava_lang_Object_Handler ()
	    {
	      if (cb_equals_Ljava_lang_Object_ == null)
	        cb_equals_Ljava_lang_Object_ = JNINativeWrapper.CreateDelegate ((Func<IntPtr, IntPtr, IntPtr, bool>) n_Equals_Ljava_lang_Object_);
	      return cb_equals_Ljava_lang_Object_;
	    }

	    static bool n_Equals_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_obj)
	    {
	      var __this = global::Java.Lang.Object.GetObject<Java.Lang.Object> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
	      var obj = global::Java.Lang.Object.GetObject<Java.Lang.Object> (native_obj, JniHandleOwnership.DoNotTransfer);
	      bool __ret = __this.Equals (obj);
	      return __ret;
	    }
	  }
	}

`Object.n_Equals_Ljava_lang_Object()` is stored in a
`Func<IntPtr, IntPtr, IntPtr, bool>` -- which conforms to JNI Native
Method Arguments -- and is then provided to
[`JNINativeWrapper.CreateDelegate()`][7], which uses
`System.Reflection.Emit` to "wrap" `n_Equals_Ljava_lang_Object()` for
exception propagation purposes.  Eventually and ultimately, when a C#
class overrides `Java.Lang.Object.Equals()`,
`Object.GetEquals_Ljava_lang_Object_Handler()` will be invoked at
runtime, and `Object.cb_equals_Ljava_lang_Object` will be stored into
`JniNativeMethodRegistration.Marshaler`.


~~ `Action<…>` and `Func<…>` ~~

There is one problem with the above approach: its use of the
`System.Action<…>` and `System.Func<…>` types used at the core of
registering native methods with JNI.  There are two problems with
using these sets of types:

 1. These delegate types only permit up to 16 parameters.  Given that
    *two* parameters are always "eaten" by the `JNIEnv*` pointer and
    a `jobject` to Java's `this` or a `jclass` to the declaring class,
    that means that we can only bind methods taking up to 14 methods.
    Java methods which take more than 14 methods are skipped.

 2. .NET Framework and CoreCLR don't support using generic types with
    the Platform Invoke marshaler and
    [`Marshal.GetFunctionPointerForDelegate()`][8].

(1) has been a longstanding problem, which we've been ignoring.

(2) isn't *yet* a problem, and is something @jonpryor has been keen
to address for awhile.


~~ C# Function Pointers? ~~

There is a proposal to [add Function Pointers to the C# language][9].
This would permit reduced overheads and improved efficiencies in
obtaining a function pointer to pass into Java code.

Unfortunately:

 1. The proposal is still ongoing, with no known release date.
 2. .NET Framework 4.x won't support them.
 3. They can't be used within the current Xamarin.Android architecture.

There doesn't appear to be a way to obtain a `Delegate` from a
`delegate*`, which means `JNINativeWrapper.CreateDelegate()` cannot
be used with Function Pointers.

In order to use Function Pointers, we would likely need to *require*
use of `tools/jnimarshalmethod-gen.exe` (176240d) so that appropriate
JNI Native Method Argument-conforming methods with the
`NativeCallableAttribute` can be generated at app build time,
*avoiding* the current Reflection-heavy registration path which
involves e.g. `Object.GetEquals_Ljava_lang_Object_Handler()`.

Unfortunately, `jnimarshalmethod-gen.exe` isn't "done": it doesn't
work on Windows, and it's use of `AppDomain`s and
`System.Reflection.Emit` look to complicate a future .NET 5 port.


~~ Solution: Generate Delegates ~~

If `Action<…>` and `Func<…>` are to be avoided, and Function Pointers
are out, how do we support more than 14 parameters?

By updating `generator` to emit the required delegate types.

When `Action<…>` or `Func<…>` would previously have been generated,
instead emit *and record the name of* a delegate which follows the
pattern:

  * Type name prefix: `_JniMarshal_PP`
  * Parameter types, using JNI encoding, e.g. `Z` for boolean,
    `I` for int, etc.  *Reference types*, normally encoded as `L…;`
    and Arrays, encoded as `[`, are each encoded as `L`.
    Kotlin unsigned types are encoded as *lower-case* forms of the
    corresponding JNI types, e.g. `i` is an unsigned `I`.
  * Another `_`.
  * The above type encoding for the return type.

For example, `Object.n_Equals_Ljava_lang_Object()` used
`Func<IntPtr, IntPtr, IntPtr, bool>`.  This would become
`_JniMarshal_PPL_Z`.

After the initial binding stage is complete and all required delegate
types are recorded, the `_JniMarshal*` types are emitted into
`__NamespaceMapping__.cs`:

	internal delegate bool _JniMarshal_PPL_Z (IntPtr jnienv, IntPtr klass, IntPtr a);

The cost to declaring all these types is that a binding assembly
contains more types.  `Mono.Android.dll`, for example, grows ~20KB
in size from all the required delegate declarations, pre-linking.


~~ Other ~~

Remove `tools/generator/generator.sln` and replace it with a
`tools/generator/generator.slnf` solution filter file which makes it
easier to work with `generator` in Visual Studio by only loading
needed projects from `Java.Interop.sln`.


[0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#compiling_loading_and_linking_native_methods
[1]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names
[2]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#RegisterNatives
[3]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#native_method_arguments
[4]: https://docs.microsoft.com/en-us/dotnet/framework/interop/marshaling-a-delegate-as-a-callback-method
[5]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getfunctionpointerfordelegate?view=netcore-3.1
[6]: https://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#equals%28java.lang.Object%29
[7]: https://github.com/xamarin/xamarin-android/blob/42822e0488185cdf4bca7c0bd05b21ad03dfbd7e/src/Mono.Android/Android.Runtime/JNINativeWrapper.cs#L34-L97
[8]: dotnet/runtime#32963
[9]: https://github.com/dotnet/csharplang/blob/master/proposals/function-pointers.md
@jpobst jpobst added this to the 10.4 (16.7 / 8.7) milestone May 11, 2020
@jpobst jpobst mentioned this pull request Sep 11, 2020
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jonpryor jonpryor jonpryor left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
11.0 (d16-7)
Development

Successfully merging this pull request may close these issues.

xamarin android binding interface method more than 16 parameter how to deal with delegate
2 participants
@jpobst @jonpryor