Separate out ABI from method bindings #795
Labels
enhancement
Proposed change to current functionality
generator
Issues binding a Java library (generator, class-parse, etc.)
Milestone
Comments
jonpryor
added
enhancement
|
This would be even better public partial class Activity {
// Metadata.xml XPath method reference: path="/api/package[@name='android.app']/class[@name='Activity']/method[@name='onCreate' and count(parameter)=1 and parameter[1][@type='android.os.Bundle']]"
partial protected virtual unsafe void OnCreate (Android.OS.Bundle? savedInstanceState);
}Activity._Binding.cs [global::Android.Runtime.Register ("android/app/Activity", DoNotGenerateAcw=true)]
public partial class Activity {
static Delegate GetOnCreate_Landroid_os_Bundle_Handler () => _JniJavaCaller.GetOnCreate_Landroid_os_Bundle_Handler ();
// Metadata.xml XPath method reference: path="/api/package[@name='android.app']/class[@name='Activity']/method[@name='onCreate' and count(parameter)=1 and parameter[1][@type='android.os.Bundle']]"
[Register ("onCreate", "(Landroid/os/Bundle;)V", "GetOnCreate_Landroid_os_Bundle_Handler")]
partial protected virtual unsafe void OnCreate (Android.OS.Bundle? savedInstanceState)
{
try {
_JniMethods.onCreate (this, savedInstanceState?.Handle ?? IntPtr.Zero);
} finally {
global::System.GC.KeepAlive (savedInstanceState);
}
}
internal static class _JniJavaCaller {
static Delegate? cb_onCreate_Landroid_os_Bundle_;
#pragma warning disable 0169
public static Delegate GetOnCreate_Landroid_os_Bundle_Handler ()
{
if (cb_onCreate_Landroid_os_Bundle_ == null)
cb_onCreate_Landroid_os_Bundle_ = JNINativeWrapper.CreateDelegate ((_JniMarshal_PPL_V) n_OnCreate_Landroid_os_Bundle_);
return cb_onCreate_Landroid_os_Bundle_;
}
static void n_OnCreate_Landroid_os_Bundle_ (IntPtr jnienv, IntPtr native__this, IntPtr native_savedInstanceState)
{
var __this = global::Java.Lang.Object.GetObject<Android.App.Activity> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!;
var savedInstanceState = global::Java.Lang.Object.GetObject<Android.OS.Bundle> (native_savedInstanceState, JniHandleOwnership.DoNotTransfer);
__this.OnCreate (savedInstanceState);
}
#pragma warning restore 0169
}
internal static class _JniMethods {
internal static readonly JniPeerMembers _members = new XAPeerMembers ("android/app/Activity", typeof (Activity));
public static void onCreate (IJavaPeerable self, IntPtr savedInstanceState)
{
const string __id = "onCreate.(Landroid/os/Bundle;)V";
JniArgumentValue* __args = stackalloc JniArgumentValue [1];
__args [0] = new JniArgumentValue (savedInstanceState);
_members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
}
}
}This would help external developers with fixing the bindings in the Metadata file. Most of the time developers don't care about the implementation, they just care about the interface. If the interface is separated out in it's own file, finding the issue by just scrolling will be easier, especially issue that aren't build errors (e.g But maybe this could be a separate issue. |
|
One issue with this proposal is that our "internal" binding infrastructure is a kind of "public" API due to our use of For example, I tried to make the trivial change to turn this generated code: Into an auto-property: However this broke While I would love all of our generated code to be cleaner, I don't know if we can do much without creating a new |
jonpryor
added a commit
to jonpryor/GenericBindingPrototype
that referenced
this issue
Mar 10, 2022
Context: https://devblogs.microsoft.com/oldnewthing/20210830-00/?p=105617 Context: dotnet/java-interop#795 It occurs to @jonpryor that what we're dealing with is *kinda* like the world of COM and COM-related technologies; see [oldnewthing][0]. > C++/WinRT and C++/CX set up parallel universes like this: > > | C++/WinRT | ABI | C++/CX | > | ----------------------- | ------------------------- | ----------------- | > | winrt::Contoso::Widget | ABI::Contoso::Widget\* | Contoso::Widget^ | We have an "ABI": what JNI expects/requires. Then we have a "projection" ("binding") from the ABI, for consumption by end users. *Historically*, we "hand-waved" over the ABI; it was *implicit* to how things worked, but *not* something that customers interacted with directly. @jonpryor would like to change that, and this seems like a reasonable "two birds, one stone" opportunity. The support for generic types, as described in `readm.md`, is contingent upon a "layer of indirection": > - Java calls the static invoker > - The static invoker calls the instance invoker > - The instance invoker calls the desired method The "instance invoker" is the "layer of indirection". The original prototype "named" this "layer of indirection" `I[Java type name]Invoker`; e.g. given Java: // Java package example; public interface ArrayList<E> { void add(E obj); } we'd have "binding + infrastructure" of: // C# namespace Example { public partial interface IArrayList<E> { void Add(E obj); } public partial interface IArrayListInvoker { void InvokeAdd(Java.Lang.Object obj); } } My proposal is to take some ideas from dotnet/java-interop#795 and WinRT projections, "formalizing" the ABI: // Convention: `jniabi.` + *Java* package name namespace jniabi.example { // Convention: Identical to Java type name public static partial class ArrayList { // Convention: Java method name + "suffix" for overloads; uses "lowest level" types. public static void add(IJavaPeerable self, JniObjectReference obj); } // Convention: `_` prefix + Java name suffix; [Register ("example/ArrayList", DoNotGenerateAcw=true)] public partial interface _ArrayList : IJavaObject, IJavaPeerable { // Convention: Java method name + "suffix" for overloads; uses "lowest level" types. [Register ("add", "(Ljava/lang/Object;)V", …); void add(JniObjectReference obj); // jni marshal method glue… private static bool n_Add_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_p0) { var __this = Java.Lang.Object.GetObject<_ArrayList>(native__this); __this!.add (new JniObjectReference (native_p0)); } } } The "un-mangled" `jniabi.example.ArrayList` type is for "manual binding" scenarios; see also xamarin-android/src/Mono.Android, which has a fair bit of "hand-written" code which was produced by taking `generator` output and copying it. `jniabi.example.ArrayList` is meant to remove the need for hand-copying code, but *only* provides low-level bindings. Actual type marshaling/etc. is "elsewhere". The "mangled" `jniabi.example._ArrayList` type is the layer of indirection, so that generic types *can* work. It's not "meant" for implementation by "normal" people, but *can* be implemented if they want/need to take control of marshaling. (For example, Photo data marshaling is like a 12MB `byte[]` that needs to be marshaled EVERY TIME, contributing to perf issues. Being able to interject and do "something else" could be *really* useful for some scenarios.) Note: wrt dotnet/java-interop#795, `_ArrayList` was instead written as `jnimarshalmethod.example.ArrayList`. I tried this initially, but wasn't "thrilled" with how it looked. Please discuss. We then separate out the "projection" ("binding"), in terms of the ABI: namespace Example { // Note: *NO* [Register]; always implements ABI public partial interface IArrayList<E> : IJavaObject, IJavaPeerable, global::jniabi.example._ArrayList { void Add (E obj); // projection // "glue" from ABI to projection; explicitly implemented void jniabi.example._ArrayList.add(JniObjectReference native_obj) { var obj = Java.Lang.Object.GetObject<T>(native_obj.Handle); Add (obj); } } } "Of course", we also want/need the raw types in the projection, if only for backward compatibility (we've historically *only* bound raw types), and this "slots in": namespace Example { // Note: *NO* [Register]; always implements ABI public partial interface IArrayList : IJavaObject, IJavaPeerable, global::jniabi.example._ArrayList { void Add (Java.Lang.Object? obj); // projection // "glue" from ABI to projection; explicitly implemented void jniabi.example._ArrayList.add(JniObjectReference native_obj) { var obj = Java.Lang.Object.GetObject<Java.Lang.Object>(native_obj.Handle); Add (obj); } } } The downside to having the raw type implement the ABI `_ArrayList` type is that it *also* incurs the indirection penalty. It *is* really flexible though! Pros to this approach vs. the "original prototype": * Reduces "namespace pollution" by moving the "indirection" types into a separate set of namespaces. I rather doubt that anyone will "accidentally" find the `jniabi.…` namespace. * Makes the ABI explicit, which in turn inserts flexibility. What if you don't *want* `java.util.Collection` to be marshaled as an `IEnumerable<T>`? With this approach, you can control it. * It's also really awesome looking to be within a generic class and use `Object.GetObject<T>()`, using the type parameter directly. * Use of Java-originated identifier names (hopefully?) reduces complexity. (I can hope, right?) Cons: * Use of Java-originated identifier names may complicate things. Sure, they're still accessible via the `generator` object model, but that doesn't mean everything is "fine". (If wishes were horses…) * "ABI" names *will* need to involve "hashes" for overloads, at some point, because all reference types "devolve" to `JniObjectReference`, so unless the number of parameters differs, method overloads that use "separate yet reference types" will collide, e.g. `java.lang.String.getBytes(String)` vs. `java.lang.String.getBytes(java.nio.charset.Charset)`. The `jniabi.java.lang.String` type will need to expose e.g. `getBytes_hash1()` and `getBytes_hash2()` methods, where the hash is e.g. the CRC for the JNI method signature. [0]: https://devblogs.microsoft.com/oldnewthing/20210830-00/?p=105617
jonpryor
added a commit
to jonpryor/GenericBindingPrototype
that referenced
this issue
Mar 14, 2022
Context: https://devblogs.microsoft.com/oldnewthing/20210830-00/?p=105617 Context: dotnet/java-interop#795 Context: dotnet/java-interop#910 It occurs to @jonpryor that what we're dealing with is *kinda* like the world of COM and COM-related technologies; see [oldnewthing][0]. > C++/WinRT and C++/CX set up parallel universes like this: > > | C++/WinRT | ABI | C++/CX | > | ----------------------- | ------------------------- | ----------------- | > | winrt::Contoso::Widget | ABI::Contoso::Widget\* | Contoso::Widget^ | We have an "ABI": what JNI expects/requires. Then we have a "projection" ("binding") from the ABI, for consumption by end users. *Historically*, we "hand-waved" over the ABI; it was *implicit* to how things worked, but *not* something that customers interacted with directly. @jonpryor would like to change that, and this seems like a reasonable "two birds, one stone" opportunity. The support for generic types, as described in `readm.md`, is contingent upon a "layer of indirection": > - Java calls the static invoker > - The static invoker calls the instance invoker > - The instance invoker calls the desired method The "instance invoker" is the "layer of indirection". The original prototype "named" this "layer of indirection" `I[Java type name]Invoker`; e.g. given Java: // Java package example; public interface ArrayList<E> { void add(E obj); } we'd have "binding + infrastructure" of: // C# namespace Example { public partial interface IArrayList<E> { void Add(E obj); } public partial interface IArrayListInvoker { void InvokeAdd(Java.Lang.Object obj); } } My proposal is to take some ideas from dotnet/java-interop#795 and WinRT projections, "formalizing" the ABI: // Convention: `jniabi.` + *Java* package name namespace jniabi.example { // Convention: `Jniabi` prefix + Java type name [EditorBrowsable (EditorBrowsableState.Never)] public static partial class JniabiArrayList { // Convention: Java method name + "suffix" for overloads; uses "lowest level" types. public static void add(IJavaPeerable self, JniObjectReference obj); } // Convention: `_Jniabi` prefix + Java name suffix; [EditorBrowsable (EditorBrowsableState.Never)] [Register ("example/ArrayList", DoNotGenerateAcw=true)] public partial interface _JniabiArrayList : IJavaObject, IJavaPeerable { // Convention: Java method name + "suffix" for overloads; uses "lowest level" types. [Register ("add", "(Ljava/lang/Object;)V", …); void add(JniObjectReference obj); // jni marshal method glue… private static bool n_Add_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_p0) { var __this = Java.Lang.Object.GetObject<_ArrayList>(native__this); __this!.add (new JniObjectReference (native_p0)); } } } [`[EditorBrowsable(EditorBrowsableState.Never)]`][1] is used to hide these types from normal IDE code completion scenarios. They're intended for internal use. The `jniabi.example.JniabiArrayList` static class is for "manual binding" scenarios; see also xamarin-android/src/Mono.Android, which has a fair bit of "hand-written" code which was produced by taking `generator` output and copying it. `jniabi.example.JniabiArrayList` is meant to remove the need for hand-copying code, but *only* provides low-level bindings. Actual type marshaling/etc. is "elsewhere". It is also useful as part of dotnet/java-interop#910, to allow "interface Invokers" to begin caching `jmethodID` values. The `jniabi.example._JniabiArrayList` interface is the layer of indirection, so that generic types *can* work. It's not "meant" for implementation by "normal" people, but *can* be implemented if they want/need to take control of marshaling. (For example, Photo data marshaling is like a 12MB `byte[]` that needs to be marshaled EVERY TIME, contributing to perf issues. Being able to interject and do "something else" could be *really* useful for some scenarios.) Note: wrt dotnet/java-interop#795, `_JniabiArrayList` was instead written as `jnimarshalmethod.example.ArrayList`. I tried this initially, but wasn't "thrilled" with how it looked. Please discuss. We then separate out the "projection" ("binding"), in terms of the ABI: namespace Example { // Note: *NO* [Register]; always implements ABI public partial interface IArrayList<E> : IJavaObject, IJavaPeerable, global::jniabi.example._ArrayList { void Add (E obj); // projection // "glue" from ABI to projection; explicitly implemented void jniabi.example._JniabiArrayList.add(JniObjectReference native_obj) { var obj = Java.Lang.Object.GetObject<T>(native_obj.Handle); Add (obj); } } } "Of course", we also want/need the raw types in the projection, if only for backward compatibility (we've historically *only* bound raw types), and this "slots in": namespace Example { // Note: *NO* [Register]; always implements ABI public partial interface IArrayList : IJavaObject, IJavaPeerable, global::jniabi.example._ArrayList { void Add (Java.Lang.Object? obj); // projection // "glue" from ABI to projection; explicitly implemented void jniabi.example._JniabiArrayList.add(JniObjectReference native_obj) { var obj = Java.Lang.Object.GetObject<Java.Lang.Object>(native_obj.Handle); Add (obj); } } } The downside to having the raw type implement the ABI `_JniabiArrayList` type is that it *also* incurs the indirection penalty. It *is* really flexible though! Pros to this approach vs. the "original prototype": * Reduces "namespace pollution" by moving the "indirection" types into a separate set of namespaces. I rather doubt that anyone will "accidentally" find the `jniabi.…` namespace. * Makes the ABI explicit, which in turn inserts flexibility. What if you don't *want* `java.util.Collection` to be marshaled as an `IEnumerable<T>`? With this approach, you can control it. * It's also really awesome looking to be within a generic class and use `Object.GetObject<T>()`, using the type parameter directly. * Use of Java-originated identifier names (hopefully?) reduces complexity. (I can hope, right?) Cons: * Use of Java-originated identifier names may complicate things. Sure, they're still accessible via the `generator` object model, but that doesn't mean everything is "fine". (If wishes were horses…) * "ABI" names *will* need to involve "hashes" for overloads, at some point, because all reference types "devolve" to `JniObjectReference`, so unless the number of parameters differs, method overloads that use "separate yet reference types" will collide, e.g. `java.lang.String.getBytes(String)` vs. `java.lang.String.getBytes(java.nio.charset.Charset)`. The `jniabi.java.lang.String` type will need to expose e.g. `getBytes_hash1()` and `getBytes_hash2()` methods, where the hash is e.g. the CRC for the JNI method signature. [0]: https://devblogs.microsoft.com/oldnewthing/20210830-00/?p=105617 [1]: https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.editorbrowsableattribute?view=net-6.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Context: https://github.com/xamarin/xamarin-android/blob/6c6766de4db50fb43c42fccf442387a95f39086c/src/Mono.Android/Android.Widget/AbsListView.cs#L55-L64
Context: dotnet/android-libraries@5ce8b5f
Background:
Sometimes it is necessary to use partial classes to "coerce" generator output to be valid C# code. A not-entirely-uncommon use case around method overrides (related: Issue #681, fixed in 5a0e37e).
Consider dotnet/android-libraries@5ce8b5f: in it,
NotificationCompat.Action.getRemoteInputs()was supposed to override a base class method, but -- for whatever reason --generatorcouldn't properly determine thatoverridewas needed. As this was before 5a0e37e, the "workaround" was to rename themanagedNameof the Java method to_GetRemoteInputs(), then add apartial classdeclaration which could have the properoverridelogic.While this works, it isn't obvious (hence #681). Additionally, such a solution should require two Metadata fixups: one to rename the method, and another to change the visibility of the method so that it isn't public:
(This wasn't done in the dotnet/android-libraries@5ce8b5f, and I don't understand why
NotificationCompat.Action._GetRemoteInputs()isn't public…)Another example is within
Mono.Android.dll(and elsewhere), where it is too common to havegeneratoremit output, copy that output into a new file, fixup that output, and then use Metadata to remove the original "bad" output. See e.g.There should be no need for such hand-written JNI code, yet we have lots of it.
Proposal
I propose that we "refactor and cleanup" our bindings to separate out the code responsible for JNI invocations from the code responsible for "C# bindings".
Consider
Android.App.Activity, where JNI code is interspersed with the C# API:I propose three changes:
jnimarshalmethods.[Java-package-name].[Java-type]class.jniabi.[Java-package-name].[Java-type]class.For example:
Key to the idea here is that
jniabi.…will contain all methods in theapi.xmlfor the declaring type. That way a partial class can always access the "ABI" methods in partial classes/etc.This may result in larger binding assemblies (all declared methods get "JNI glue code", not just those appearing within the bindings). This assumes that the linker will be able to remove such unused methods, which it should be able to.
Related design points on the
jniabitype:IJavaObject.The text was updated successfully, but these errors were encountered: