From 950a40a80ba8d32fdeaecd8097b70852c1176d7d Mon Sep 17 00:00:00 2001 From: Lukas Stadler <lukas.stadler@oracle.com> Date: Tue, 4 Apr 2017 19:23:40 +0200 Subject: [PATCH] do not validate JNI refs by default, use fast IsSameElement --- .../fficall/src/jni/rffiutils.c | 31 ++++++++++--- .../fficall/src/jni/rffiutils.h | 2 +- com.oracle.truffle.r.native/version.source | 2 +- .../truffle/r/runtime/ffi/jni/JNI_Call.java | 45 +++++++++++++------ 4 files changed, 59 insertions(+), 21 deletions(-) diff --git a/com.oracle.truffle.r.native/fficall/src/jni/rffiutils.c b/com.oracle.truffle.r.native/fficall/src/jni/rffiutils.c index 2e74a0498f..d7a786a49a 100644 --- a/com.oracle.truffle.r.native/fficall/src/jni/rffiutils.c +++ b/com.oracle.truffle.r.native/fficall/src/jni/rffiutils.c @@ -85,7 +85,7 @@ void setEmbedded() { #ifdef TRACE_ENABLED // Helper for debugging purposes: prints out (into the trace file) the java // class name for given SEXP -static const char* fastRInspect(JNIEnv *env, SEXP v) { +static void fastRInspect(JNIEnv *env, SEXP v) { // this invokes getClass().getName() jclass cls = (*env)->GetObjectClass(env, v); jmethodID getClassMethodID = checkGetMethodID(env, cls, "getClass", "()Ljava/lang/Class;", 0); @@ -104,7 +104,24 @@ static const char* fastRInspect(JNIEnv *env, SEXP v) { #endif static int isValidJNIRef(JNIEnv *env, SEXP ref) { +#if VALIDATE_REFS return (*env)->GetObjectRefType(env, ref) != JNIInvalidRefType; +#else + return TRUE; +#endif +} + +static jboolean fast_IsSameObject(jobject a, jobject b) { + // this takes some assumptions about jni handles, but it is much faster + void** pA = (void**) a; + void** pB = (void**) b; + if (pA == NULL && pB == NULL) { + return TRUE; + } else if (pA == NULL || pB == NULL) { + return FALSE; + } else { + return *pA == *pB; + } } // native down call depth, indexes nativeArrayTableHwmStack @@ -193,7 +210,7 @@ void invalidateNativeArray(JNIEnv *env, SEXP oldObj) { assert(isValidJNIRef(env, oldObj)); for (int i = 0; i < nativeArrayTableHwm; i++) { NativeArrayElem cv = nativeArrayTable[i]; - if ((*env)->IsSameObject(env, cv.obj, oldObj)) { + if (fast_IsSameObject(cv.obj, oldObj)) { #if TRACE_NATIVE_ARRAYS fprintf(traceFile, "invalidateNativeArray(%p): found\n", oldObj); #endif @@ -219,7 +236,7 @@ void updateNativeArrays(JNIEnv *env) { static void *findNativeArray(JNIEnv *env, SEXP x) { if (nativeArrayTableLastIndex < nativeArrayTableHwm) { NativeArrayElem cv = nativeArrayTable[nativeArrayTableLastIndex]; - if (cv.obj != NULL && (cv.obj == x || (*env)->IsSameObject(env, cv.obj, x))) { + if (cv.obj != NULL && (cv.obj == x || fast_IsSameObject(cv.obj, x))) { void *data = cv.data; #if TRACE_NATIVE_ARRAYS fprintf(traceFile, "findNativeArray(%p): found %p (cached)\n", x, data); @@ -233,7 +250,7 @@ static void *findNativeArray(JNIEnv *env, SEXP x) { NativeArrayElem cv = nativeArrayTable[i]; if (cv.obj != NULL) { assert(isValidJNIRef(env, cv.obj)); - if ((*env)->IsSameObject(env, cv.obj, x)) { + if (fast_IsSameObject(cv.obj, x)) { nativeArrayTableLastIndex = i; void *data = cv.data; #if TRACE_NATIVE_ARRAYS @@ -409,7 +426,7 @@ static SEXP findCachedGlobalRef(JNIEnv *env, SEXP obj) { if (elem.gref == NULL) { continue; } - if ((*env)->IsSameObject(env, elem.gref, obj)) { + if (fast_IsSameObject(elem.gref, obj)) { #if TRACE_REF_CACHE fprintf(traceFile, "gref: cache hit: %d\n", i); #endif @@ -470,7 +487,7 @@ void releaseGlobalRef(JNIEnv *env, SEXP obj) { if (elem.gref == NULL || elem.permanent) { continue; } - if ((*env)->IsSameObject(env, elem.gref, obj)) { + if (fast_IsSameObject(elem.gref, obj)) { #if TRACE_REF_CACHE fprintf(traceFile, "gref: release: index %d, gref: %p\n", i, elem.gref); #endif @@ -481,12 +498,14 @@ void releaseGlobalRef(JNIEnv *env, SEXP obj) { } void validateRef(JNIEnv *env, SEXP x, const char *msg) { +#ifdef VALIDATE_REFS jobjectRefType t = (*env)->GetObjectRefType(env, x); if (t == JNIInvalidRefType) { char buf[1000]; sprintf(buf, "%s %p", msg,x); fatalError(buf); } +#endif } JNIEnv *getEnv() { diff --git a/com.oracle.truffle.r.native/fficall/src/jni/rffiutils.h b/com.oracle.truffle.r.native/fficall/src/jni/rffiutils.h index 2af57ebd82..e2dc370a32 100644 --- a/com.oracle.truffle.r.native/fficall/src/jni/rffiutils.h +++ b/com.oracle.truffle.r.native/fficall/src/jni/rffiutils.h @@ -31,7 +31,7 @@ #include <setjmp.h> #include <Connections.h> -#define VALIDATE_REFS 1 +#define VALIDATE_REFS 0 JNIEnv *getEnv(); void setEnv(JNIEnv *env); diff --git a/com.oracle.truffle.r.native/version.source b/com.oracle.truffle.r.native/version.source index d6b24041cf..209e3ef4b6 100644 --- a/com.oracle.truffle.r.native/version.source +++ b/com.oracle.truffle.r.native/version.source @@ -1 +1 @@ -19 +20 diff --git a/com.oracle.truffle.r.runtime.ffi/src/com/oracle/truffle/r/runtime/ffi/jni/JNI_Call.java b/com.oracle.truffle.r.runtime.ffi/src/com/oracle/truffle/r/runtime/ffi/jni/JNI_Call.java index f729327eba..115c562a7d 100644 --- a/com.oracle.truffle.r.runtime.ffi/src/com/oracle/truffle/r/runtime/ffi/jni/JNI_Call.java +++ b/com.oracle.truffle.r.runtime.ffi/src/com/oracle/truffle/r/runtime/ffi/jni/JNI_Call.java @@ -59,20 +59,39 @@ public class JNI_Call implements CallRFFI { } try { switch (args.length) { - // @formatter:off - case 0: result = call0(address); break; - case 1: result = call1(address, args[0]); break; - case 2: result = call2(address, args[0], args[1]); break; - case 3: result = call3(address, args[0], args[1], args[2]); break; - case 4: result = call4(address, args[0], args[1], args[2], args[3]); break; - case 5: result = call5(address, args[0], args[1], args[2], args[3], args[4]); break; - case 6: result = call6(address, args[0], args[1], args[2], args[3], args[4], args[5]); break; - case 7: result = call7(address, args[0], args[1], args[2], args[3], args[4], args[5], args[6]); break; - case 8: result = call8(address, args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7]); break; - case 9: result = call9(address, args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8]); break; + case 0: + result = call0(address); + break; + case 1: + result = call1(address, args[0]); + break; + case 2: + result = call2(address, args[0], args[1]); + break; + case 3: + result = call3(address, args[0], args[1], args[2]); + break; + case 4: + result = call4(address, args[0], args[1], args[2], args[3]); + break; + case 5: + result = call5(address, args[0], args[1], args[2], args[3], args[4]); + break; + case 6: + result = call6(address, args[0], args[1], args[2], args[3], args[4], args[5]); + break; + case 7: + result = call7(address, args[0], args[1], args[2], args[3], args[4], args[5], args[6]); + break; + case 8: + result = call8(address, args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7]); + break; + case 9: + result = call9(address, args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8]); + break; default: - result = call(address, args); break; - // @formatter:on + result = call(address, args); + break; } return result; } finally { -- GitLab