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

FunctionPointer performance suffers due to repeated AttachCurrentThread/DetachCurrentThread #242

Closed
tmm1 opened this issue Jul 6, 2018 · 7 comments · Fixed by #243
Closed

Comments

@tmm1
Copy link
Contributor

tmm1 commented Jul 6, 2018

What is the difference between implementing call() vs apply() on a FunctionPointer?

I've been using call(), and I noticed that everytime my C code is invoking the function callback, it seems to be run in a brand new Java thread. Would using apply() instead make a difference?

@tmm1
Copy link
Contributor Author

tmm1 commented Jul 6, 2018

Looks like call and apply are the same:

} else if (methodName.startsWith("call") || methodName.startsWith("apply")) {
// found a function caller method and/or callback method
functionMethods[0] = methods[i];

I'm still curious why/where the new thread is being created..

@tmm1
Copy link
Contributor Author

tmm1 commented Jul 6, 2018

I noticed that everytime my C code is invoking the function callback, it seems to be run in a brand new Java thread

Looks like this was discussed previously in #143

There's a hack available as of 3abc7bf to prevent the attach/detach per callback invocation, however that's not ideal as it provides no way to eventually detach the JNI env.

The recommended way to implement this on Android is by using pthread_setspecific to ensure the JNI context is detached when the pthread dies. See for example https://github.com/FFmpeg/FFmpeg/blob/cced03dd667a5df6df8fd40d8de0bff477ee02e8/libavcodec/ffjni.c#L34-L95

@tmm1
Copy link
Contributor Author

tmm1 commented Jul 6, 2018

There's a hack available as of 3abc7bf to prevent the attach/detach per callback invocation, however that's not ideal as it provides no way to eventually detach the JNI env.

Though that commit states resources may leak if the Detach isn't invoked, the behavior on Android is that the entire process is killed if any thread dies without cleaning up its JNI env.

@saudet
Copy link
Member

saudet commented Jul 6, 2018 via email

@tmm1
Copy link
Contributor Author

tmm1 commented Jul 6, 2018

Yea I think that would be ideal. If the Attach works, then setup the pthread destructor to detach when the native thread dies. That's all that would be required, and it would work seamlessly.

@saudet
Copy link
Member

saudet commented Jul 6, 2018 via email

@tmm1 tmm1 changed the title FunctionPointer call vs apply FunctionPointer performance suffers due to repeated AttachCurrentThread/DetachCurrentThread Jul 7, 2018
@tmm1
Copy link
Contributor Author

tmm1 commented Jul 7, 2018

Okay I can take a stab.

Here are some more resources that talk about how DetachCurrentThread is required and the thread-specific destructor technique for detaching:

https://stackoverflow.com/a/26534926/332798

https://developer.android.com/training/articles/perf-jni#threads

And another discussion of why Attach/Detach repeatedly is bad for performance, specifically in the context of a C callback:

https://groups.google.com/forum/m/#!topic/android-ndk/8y38l_IqrQ0

Finally see FFmpeg/FFmpeg@376d8fb where the repeated Attach/Detach was also removed due to performance problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants