diff --git a/java/jni/YGJNI.cpp b/java/jni/YGJNI.cpp index 9b7a7dad..8fae6d4c 100644 --- a/java/jni/YGJNI.cpp +++ b/java/jni/YGJNI.cpp @@ -719,7 +719,7 @@ jint jni_YGNodeGetInstanceCount() { } #define YGMakeNativeMethod(name) makeNativeMethod(#name, name) -#define YGMakeCriticalNativeMethod(name) makeCriticalNativeMethod(#name, name) +#define YGMakeCriticalNativeMethod(name) makeCriticalNativeMethod_DO_NOT_USE_OR_YOU_WILL_BE_FIRED(#name, name) jint JNI_OnLoad(JavaVM* vm, void*) { return initialize(vm, [] { diff --git a/lib/fb/src/main/cpp/include/fb/fbjni/CoreClasses.h b/lib/fb/src/main/cpp/include/fb/fbjni/CoreClasses.h index 227e9c79..09fa8b82 100644 --- a/lib/fb/src/main/cpp/include/fb/fbjni/CoreClasses.h +++ b/lib/fb/src/main/cpp/include/fb/fbjni/CoreClasses.h @@ -234,9 +234,9 @@ class FBEXPORT JClass : public JavaClass { /// makeNativeMethod("nativeMethodWithExplicitDescriptor", /// "(Lcom/facebook/example/MyClass;)V", /// methodWithExplicitDescriptor), - /// makeCriticalNativeMethod("criticalNativeMethodWithAutomaticDescriptor", + /// makeCriticalNativeMethod_DO_NOT_USE_OR_YOU_WILL_BE_FIRED("criticalNativeMethodWithAutomaticDescriptor", /// criticalNativeMethodWithAutomaticDescriptor), - /// makeCriticalNativeMethod("criticalNativeMethodWithExplicitDescriptor", + /// makeCriticalNativeMethod_DO_NOT_USE_OR_YOU_WILL_BE_FIRED("criticalNativeMethodWithExplicitDescriptor", /// "(IIF)Z", /// criticalNativeMethodWithExplicitDescriptor), /// }); diff --git a/lib/fb/src/main/cpp/include/fb/fbjni/Registration.h b/lib/fb/src/main/cpp/include/fb/fbjni/Registration.h index 7545840e..0c2d6eac 100644 --- a/lib/fb/src/main/cpp/include/fb/fbjni/Registration.h +++ b/lib/fb/src/main/cpp/include/fb/fbjni/Registration.h @@ -96,6 +96,55 @@ struct CriticalMethod { // signature with an exclamation mark. // Android v8+ supports fast calls by annotating methods: // https://source.android.com/devices/tech/dalvik/improvements#faster-native-methods +// +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// +// "Fast" calls are only on the order of a few dozen NANO-seconds faster than +// regular JNI calls. If your method does almost aaanything of consequence - if +// you loop, if you write to a log, if you call another method, if you even +// simply allocate or deallocate - then the method body will significantly +// outweigh the method overhead. +// +// The difference between a regular JNI method and a "FastJNI" method (as +// they're called inside the runtime) is that a FastJNI method doesn't mark the +// thread as executing native code, and by skipping that avoids the locking and +// thread state check overhead of interacting with the Garbage Collector. +// +// To understand why this is dangerous, you need to understand a bit about the +// GC. In order to perform its work the GC needs to have at least one (usually +// two in modern implementations) "stop the world" checkpoints where it can +// guarantee that all managed-code execution is paused. The VM performs these +// checkpoints at allocations, method boundaries, and each backward branch (ie +// anytime you loop). When the GC wants to run, it will signal to all managed +// threads that they should pause at the next checkpoint, and then it will wait +// for every thread in the system to transition from the "runnable" state into a +// "waiting" state. Once every thread has stopped, the GC thread can perform the +// work it needs to and then it will trigger the execution threads to resume. +// +// JNI methods fit neatly into the above paradigm: They're still methods, so +// they perform GC checkpoints at method entry and method exit. JNI methods also +// perform checkpoints at any JNI boundary crossing - ie, any time you call +// GetObjectField etc. Because access to managed objects from native code is +// tightly controlled, the VM is able to mark threads executing native methods +// into a special "native" state which the GC is able to ignore: It knows they +// can't touch managed objects (without hitting a checkpoint) so it doesn't care +// about them. +// +// JNI critical methods don't perform that "runnable" -> "native" thread state +// transition. Skipping that transition allows them to shave about 20ns off +// their total execution time, but it means that the GC has to wait for them to +// complete before it can move forward. If a critical method begins blocking, +// say on a long loop, or an I/O operation, or on perhaps a mutex, then the GC +// will also block, and because the GC is blocking the entire rest of the VM +// (which is waiting on the GC) will block. If the critical method is blocking +// on a mutex that's already held by the GC - for example, the VM's internal +// weak_globals_lock_ which guards modifications to the weak global reference +// table (and is required in order to create or free a weak_ref<>) - then you +// have a system-wide deadlock. // prefixes a JNI method signature as android "fast call". #if defined(__ANDROID__) && defined(FBJNI_WITH_FAST_CALLS) @@ -106,20 +155,25 @@ struct CriticalMethod { #define makeCriticalNativeMethod3(name, desc, func) \ makeNativeMethod3( \ - name, \ - FBJNI_PREFIX_FAST_CALL(desc), \ - ::facebook::jni::detail::CriticalMethod::call<&func>) + name, \ + FBJNI_PREFIX_FAST_CALL(desc), \ + ::facebook::jni::detail::CriticalMethod::call<&func>) -#define makeCriticalNativeMethod2(name, func) \ - makeCriticalNativeMethod3( \ - name, \ - ::facebook::jni::detail::CriticalMethod::desc<&func>(), \ - func) +#define makeCriticalNativeMethod2(name, func) \ + makeCriticalNativeMethod3( \ + name, \ + ::facebook::jni::detail::CriticalMethod::desc<&func>(), \ + func) -#define makeCriticalNativeMethodN(a, b, c, count, ...) \ - makeCriticalNativeMethod##count -#define makeCriticalNativeMethod(...) \ - makeCriticalNativeMethodN(__VA_ARGS__, 3, 2)(__VA_ARGS__) +#define makeCriticalNativeMethodN(a, b, c, count, ...) makeCriticalNativeMethod ## count + +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// See above for an explanation. +#define makeCriticalNativeMethod_DO_NOT_USE_OR_YOU_WILL_BE_FIRED(...) makeCriticalNativeMethodN(__VA_ARGS__, 3, 2)(__VA_ARGS__) }}