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

JNI local / global references #805

Closed
mahesh-hegde opened this issue Jul 12, 2022 · 3 comments
Closed

JNI local / global references #805

mahesh-hegde opened this issue Jul 12, 2022 · 3 comments
Assignees

Comments

@mahesh-hegde
Copy link
Contributor

Current design of the utility library package:jni assumes that straight line code is executed in the same thread, which is not actually a guarantee made by Dart.

The current test cases are fairly short and we cannot rely on this assumption in large packages / apps with various interacting parts, or as the upstream dart implementation changes. Therefore the only reliable way seems to migrate everything in package:jni to use global references, and abstract the JNIEnv as well, since it's per-thread in JNI.

This necessitates some changes in package:jni

First, some context.

JNIEnv and autogenerated methods on Pointer<JNIEnv>

JNIEnv is valid in the thread it's obtained in. However if 2 statements in straight line code may actually be scheduled on different threads, it's impossible to use JNIEnv directly from dart.

Instead of accessing member functions like now we are, we have to create C functions for each of them, which obtains JNIEnv for the thread if not already attached, (In C we can use thread_local or __declspec(thread) as we do now, and C code runs continuously).

If we want to provide an equivalent for every JNI function, then we have to autogenerate a method for each function pointer in the JNIEnv table, in C, and generate a link to that.

There are 2 ways to expose our C-wrapped functions: one is creating a shadow struct with all the pointers and exposing that. Or create a top level function in header and leave the rest to FFIGEN.

Local and global references

Every function on JNIEnv returns local reference except a few of them. however when a function takes a JObject it doesn't make a difference if ref is local or global.

Also this requires us to move the convenience methods in jni_object.dart to C as much as possible.

Sustainability of large number of global references

Be careful using global references. Global references can be unavoidable, but they are difficult to debug and can cause difficult-to-diagnose memory (mis)behaviors. All else being equal, a solution with fewer global references is probably better.

https://developer.android.com/training/articles/perf-jni#local-and-global-references

It still remains to be seen what's the performance impact. If the performance impact is really bad, we might want some thread pinning in the SDK instead (dart-lang/sdk#46943) or some other form of guarantee.

cc: @dcharkes @liamappelbe

@mahesh-hegde mahesh-hegde self-assigned this Jul 12, 2022
@dcharkes
Copy link
Collaborator

Thanks for the write-up!

Assuming that our current approach "mostly-works" because we're almost always on the same thread in practise. Is it possible to postpone this refactoring till after we create the code generator? (And we then retarget the code generator later. That would enable us to get some results earlier and investigate the thread issue in parallel.)

@mahesh-hegde
Copy link
Contributor Author

I believe that's possible.

For jni_gen code, we need to create a new wrapper class as base of JNI objects [1], which uses global references. We have to generate C code anyway. C will do most heavy lifting, so that temporaries etc.. can be local references.

[1] we need to do this anyway, since JniObject wrapper in package:jni brings in lot of utility methods with it. I would personally like something lightweight, which can be then converted into JniObject if we want to manually invoke methods / access fields.

@mahesh-hegde
Copy link
Contributor Author

Addressed sufficiently in dart-archive/jnigen#53 .

@HosseinYousefi HosseinYousefi transferred this issue from dart-archive/jnigen Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants