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.

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

[jnigen] Generate ResourceIdentifiers #1098

Open
dcharkes opened this issue Apr 23, 2024 · 3 comments
Open

[jnigen] Generate ResourceIdentifiers #1098

dcharkes opened this issue Apr 23, 2024 · 3 comments

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Apr 23, 2024

To address #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:

@dcharkes
Copy link
Collaborator Author

@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',
})

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Apr 24, 2024

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
Copy link
Member

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 HosseinYousefi added this to the JNI / JNIgen 0.10.0 milestone Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

2 participants