Page MenuHomePhabricator

Implement C++ class to pass Java lambda (Runnable interface implementation) to C++ code)
ClosedPublic

Authored by marcin on Sep 27 2022, 4:58 AM.
Tags
None
Referenced Files
F3368425: D5235.id.diff
Mon, Nov 25, 7:48 PM
Unknown Object (File)
Mon, Nov 11, 4:25 AM
Unknown Object (File)
Sat, Nov 9, 8:11 PM
Unknown Object (File)
Sat, Nov 9, 8:11 PM
Unknown Object (File)
Sat, Nov 9, 8:11 PM
Unknown Object (File)
Sat, Nov 9, 8:11 PM
Unknown Object (File)
Sat, Nov 9, 8:11 PM
Unknown Object (File)
Sat, Nov 9, 8:11 PM
Subscribers

Details

Summary

This differential implements JNI class to pass lambda between C++ and Java. It will be needed to use GlobalDBSingleton API in Java via JNI.

Test Plan

Hard to provide any valuable test procedure without corresponding Java code that would pass lambda to C++ with the help of this class.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

doesn't look wrong, but not familiar enough with jni to know if it's right. LGTM

tomek added inline comments.
native/android/app/src/cpp/jniHelpers.h
26 ↗(On Diff #17104)

Why do we need to find static class? Can't we use getClass() like in HashMap?

This revision is now accepted and ready to land.Sep 28 2022, 3:57 AM
native/android/app/src/cpp/jniHelpers.h
26 ↗(On Diff #17104)

I had a discussion with expo team member from SWM about this matter. I was told that findClassStatic finds base class method (interface Runnable) while getClass() finds implementation of derived class (our lambda). So we can use getClass() but it cannot be declared as static since it would fail after using different lambda since it would remember implementation of the previous one. I opted for static and findClassStatic since I was told it can be a little faster.

native/android/app/src/cpp/jniHelpers.h
26 ↗(On Diff #17104)

That makes sense! In that case, maybe we should something similar for the HashMap - and take a class definition of AbstractMap.

native/android/app/src/cpp/jniHelpers.h
26 ↗(On Diff #17104)

It is not the findClassStatic being faster than getClass but the usage of static keyword. With static the function is called only once. I would not make a difference in speed for HashMap.

native/android/app/src/cpp/jniHelpers.h
26 ↗(On Diff #17104)

That's interesting. But wouldn't that mean that HashMap would fail if the first usage was with a subclass of java.util.HashMap and the second with java.util.HashMap?

native/android/app/src/cpp/jniHelpers.h
26 ↗(On Diff #17104)

With static declaration - yes.

We should be really sure about this code. If there are any issues, it will probably cause crashes we don't know about. I encourage everybody on this diff to be extremely thorough. We should be 100% sure of every line here. We should not be guessing, or trying things that seem to work without understanding 100% of why/how they work.