native icon indicating copy to clipboard operation
native copied to clipboard

[jnigen] Generate `ResourceIdentifier`s

Open dcharkes opened this issue 1 year ago • 3 comments
trafficstars

To address https://github.com/dart-lang/native/issues/681, we should to generate ResourceIdentifiers on all JNIgen generated methods.

class PDDocument extends jni.JObject {

  @ResourceIdentifier('jnigen')
  jni.JObject getDocumentCatalog();

  @ResourceIdentifier('jnigen')
  static PDDocument load(
    jni.JObject file,
  );

}

The consuming of the annotations can be tested with pkg/vm/tool/gen_kernel. pkg/vm/tool/gen_kernel --help shows the API. --resources-file=... --aot --tfa are the arguments to use.

Some questions to answer:

  • What do we need from the resources.json format
    • Currently the class surrounding the method is missing (workaround: ResourceIdentifier('PDDocument') to get the class this way)
    • Instance methods are not supported. What would we need for instance methods? The identical metadata in the annotation on every override? Some serialization of the class hierarchy?
  • Do we possibly need to change the structure of the generated JNI API?

Some assumptions:

  • We cannot generate proguard rules for any dynamic invocations.
  • We cannot generate proguard rules for uses of package:jni invokeMethod and friends.

@HosseinYousefi This would be very useful to know in the design of hook/link.dart and the resources.json format. Can you give the existing implementation a spin and try to answer the above questions? (And maybe ask some more good questions! 😄)

Related issues:

  • https://github.com/dart-lang/native/issues/1085
  • https://github.com/dart-lang/native/issues/1099

dcharkes avatar Apr 23 '24 20:04 dcharkes

@liamappelbe mentioned that we'd need to deal with renames and super classes, so probably we should generate something along the lines of:

@ResourceIdentifier({
  'jnigen': true,
  'java_class' : 'SomeClass',
  'java_method' : 'someMethod',
  'java_method_static' : true,
  'java_super_class' : 'SomeOtherClass',
})

dcharkes avatar Apr 24 '24 07:04 dcharkes

Instance methods are not supported.

We have static final method IDs for all methods, and static final class object as well. So we could generate the resource identifiers for those instead.

  • Do we possibly need to change the structure of the generated JNI API?

I don't think so, I will try it!

Some assumptions:

  • We cannot generate proguard rules for any dynamic invocations.

Of course, this seems to be fine. If you want any level of dynamicism, you'll have to deal with keeping the classes and methods yourself.

  • We cannot generate proguard rules for uses of package:jni invokeMethod and friends.

We could consider supporting this use case iff all the names and signatures are known at compile time. And maybe issue a helpful warning when the names and signatures are not const.

final jclass = JClass.forName('Ljava/nio/ByteBuffer;'); // using a string literal, so we could keep it
const name = 'allocate';
final allocate = jclass.staticMethodId(name, '(I)Ljava/nio/ByteBuffer;'); // ditto

Will we be able to "link" the method name with the class name though? Or does the JClass need to be const as well?

I can construct another use case, imagine we have a bunch of icon fonts in a Flutter application and we want to access a certain one:

final font = Font('assets/fonts/1.ttf');
final icon = font.icon(5);
// We would ideally like to keep the icon number 5 from 1.ttf and tree-shake everything else that's unused.

we'd need to deal with renames and super classes

We only need the class' binary name, member name, its signature, and its modifiers to keep the class or member in proguard: https://www.guardsquare.com/manual/configuration/usage

HosseinYousefi avatar Apr 24 '24 10:04 HosseinYousefi

We could consider supporting this use case iff all the names and signatures are known at compile time.

For example using @mustBeConst.

cc/ @mosuem

HosseinYousefi avatar Apr 24 '24 15:04 HosseinYousefi