java.interop icon indicating copy to clipboard operation
java.interop copied to clipboard

Emit [DynamicDependency] to preserve Invokers

Open jonpryor opened this issue 1 year ago • 8 comments

In the current .NET for Android world order, the .NET for Android workload adds custom linker steps to preserve the *Invoker types when a bound Java type is preserved.

In a possible future NativeAOT world order, there are no custom linker steps. We thus must use the linker-supported custom attributes to cause types to be preserved.

Given:

partial interface IMyInterface : IJavaPeerable {
    // …
}
partial class IMyInterfaceInvoker : Java.Lang.Object, IMyInterface {
    // …
}

We should place [DynamicDependency] on the interface (and/or abstract class) declaration so that the linker knows to preserve the Invoker:

[DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(IMyInterfaceInvoker))]
partial interface IMyInterface : IJavaPeerable {
    // …
}
partial class IMyInterfaceInvoker : Java.Lang.Object, IMyInterface {
    // …
}

jonpryor avatar Sep 30 '24 23:09 jonpryor

[DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(IMyInterfaceInvoker))]
partial interface IMyInterface : IJavaPeerable {
    // …
}
partial class IMyInterfaceInvoker : Java.Lang.Object, IMyInterface {
    // …
}

This is not valid, as [DynamicDependency] can only be placed on a Constructor/Field/Method.

Is there another way to accomplish this? Or do we need to add this attribute to every type member? 🤮

jpobst avatar Dec 03 '24 20:12 jpobst

How do we get from the interface to the invoker at runtime? What do we do with the invoker once we have it?

My general answer will always be: "Don't use DynamicDependency. Instead figure out how we use reflection and adapt to that, ideally removing the reflection and if not possible make it more declarative"

vitek-karas avatar Dec 04 '24 10:12 vitek-karas

@vitek-karas: please see this for a reasonable overview: 1adb7964a2033c83c298c070f2d1ab896d92671b

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 ();
}

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?

How it works is via convention and Reflection.

How do we get from the interface to the invoker at runtime?

We append "Invoker" to the abstract class or interface name, and resolve it from the same assembly:

  • https://github.com/dotnet/android/blob/65906e0b7b2f471fcfbd07e7e01b68169c25d9da/src/Mono.Android/Java.Interop/JavaObjectExtensions.cs#L110-L147
  • https://github.com/dotnet/java-interop/blob/65906e0b7b2f471fcfbd07e7e01b68169c25d9da/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs#L386-L424

What do we do with the invoker once we have it?

We instantiate it:

  • https://github.com/dotnet/android/blob/b07a0c9e9cd023666b7c6a1bdbc0ebd3eeffb169/src/Mono.Android/Java.Interop/TypeManager.cs#L286
  • https://github.com/dotnet/android/blob/b07a0c9e9cd023666b7c6a1bdbc0ebd3eeffb169/src/Mono.Android/Java.Interop/TypeManager.cs#L321
  • https://github.com/dotnet/android/blob/b07a0c9e9cd023666b7c6a1bdbc0ebd3eeffb169/src/Mono.Android/Java.Interop/TypeManager.cs#L337-L362
  • https://github.com/dotnet/java-interop/blob/65906e0b7b2f471fcfbd07e7e01b68169c25d9da/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs#L324

ideally removing the reflection and if not possible make it more declarative"

I'm not sure I understand what that means. My guess is that you're suggesting we e.g. update [Register] to contain a Type directly referencing the Invoker type, e.g.

partial class RegisterAttribute : Attribute {
    [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)]
    public Type? InvokerType {get; set;}
}

which would allow:

[Register (…, InvokerType=typeof(IRunnableInvoker))]
public partial interface IRunnable {
}

internal partial class IRunnableInvoker {
}

jonpryor avatar Dec 04 '24 12:12 jonpryor

The InvokerType property is apparently the right idea: https://discord.com/channels/732297728826277939/732297837953679412/1313852503070212146

jonpryor avatar Dec 04 '24 13:12 jonpryor

@jpobst: okay, so this got more complicated. :-(

Firstly, we need to update our attributes:

namespace Java.Interop {
    // In java-interop
    public partial class JniTypeSignatureAttribute {
        [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)]
        public Type? InvokerType {get; set;}
    }
}

// in dotnet/android
namespace Android.Runtime {
    partial class RegisterAttribute : Attribute {
        [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)]
        public Type? InvokerType {get; set;}
    }
}

Then we need to update generator to include InvokerType=typeof(WhateverInvoker) in the [JniTypeSignature] and [Register] output.

Then we need to update Java.Interop.dll & Mono.Android.dll to now look for and use InvokerType instead/in addition to the "append Invoker" convention we currently use.

Java.Interop.dll could probably be updated to only support InvokerType.

@jpobst: open question: should we update [Register] at all? Or just have XAJavaInterop1 start emitting [JniTypeSignature]?

jonpryor avatar Dec 04 '24 13:12 jonpryor

I would also consider not using All unless we absolutely have to. All is kind of weird in that it will keep really everything about the type, so for example if the type implements an interface it will force all the methods on that interface to be kept (for everybody) and so on. It seems this is consumed by the CreateProxy which only needs all constructors.

vitek-karas avatar Dec 04 '24 13:12 vitek-karas

@vitek-karas wrote:

for example if the type implements an interface it will force all the methods on that interface to be kept (for everybody) and so on.

I think that's mostly what we actually want/need. (See also.) For interfaces, all interface members must be preserved, always, because Java might call them, and we can't know whether or not that'll actually happen.

Abstract classes are more difficult, as we do want the linker to be able to remove non-overridden methods. However, if user code overrides a Java method, that must be preserved, especially when the only callers of that method are from Java.

You're thus immediately right that a new InvokerType property should constrain to PublicConstructors | NonPublicConstructors, but I believe that won't be enough, and I'm not sure what "enough" would be.

jonpryor avatar Dec 04 '24 17:12 jonpryor

@jpobst: open question: should we update [Register] at all? Or just have XAJavaInterop1 start emitting [JniTypeSignature]?

Given that we already know there are issues with using [JniTypeSignature] for XAJavaInterop1 and that .NET 10 is likely going to have a lot of risk due to the other major changes we plan on making, I think my preference would be to update [Register] and not make too many changes at one.

If switching to [JniTypeSignature] is something we deem important in the .NET 10 timeframe we should probably tackle it separately.

jpobst avatar Dec 05 '24 17:12 jpobst