Move array pinning out of global references table.
We have to pin primitive arrays for certain JNI operations (it's that or
return a copy of the contents). We don't move objects around in the
virtual heap, so simply creating a global reference is enough to ensure
the correct behavior of the calls.
The global reference implementation is changing in a way that makes
this approach inconvenient, so we now have a separate table for pinned
primitive arrays.
As a bonus, if GREF tracking is enabled, we will scan through the table
when a pin entry is added. If there are 10 or more entries for the same
array, a complaint is logged. This should allow us to find mismatched
Get/Release sequences much more easily (before you had to wait until the
global reference table exploded).
We currently scan pin table entries at the same time as the JNI global
references, so they'll still show up in hprof dumps as such. (Could
also add a new Android-specific hprof root category, but that seems like
more trouble than it's worth.)
diff --git a/vm/Globals.h b/vm/Globals.h
index 2ac1187..a1a1b83 100644
--- a/vm/Globals.h
+++ b/vm/Globals.h
@@ -415,6 +415,12 @@
int jniGlobalRefLoMark;
/*
+ * JNI pinned object table (used for primitive arrays).
+ */
+ ReferenceTable jniPinRefTable;
+ pthread_mutex_t jniPinRefLock;
+
+ /*
* Native shared library table.
*/
HashTable* nativeLibs;
diff --git a/vm/Jni.c b/vm/Jni.c
index 0a8103b..4df32b1 100644
--- a/vm/Jni.c
+++ b/vm/Jni.c
@@ -349,9 +349,12 @@
#define kGlobalRefsTableInitialSize 512
#define kGlobalRefsTableMaxSize 51200 /* arbitrary, must be < 64K */
#define kGrefWaterInterval 100
-
#define kTrackGrefUsage true
+#define kPinTableInitialSize 16
+#define kPinTableMaxSize 1024
+#define kPinComplainThreshold 10
+
/*
* Allocate the global references table, and look up some classes for
* the benefit of direct buffer access.
@@ -363,10 +366,15 @@
return false;
dvmInitMutex(&gDvm.jniGlobalRefLock);
-
gDvm.jniGlobalRefLoMark = 0;
gDvm.jniGlobalRefHiMark = kGrefWaterInterval * 2;
+ if (!dvmInitReferenceTable(&gDvm.jniPinRefTable,
+ kPinTableInitialSize, kPinTableMaxSize))
+ return false;
+
+ dvmInitMutex(&gDvm.jniPinRefLock);
+
/*
* Look up and cache pointers to some direct buffer classes, fields,
* and methods.
@@ -799,29 +807,74 @@
* Objects don't currently move, so we just need to create a reference
* that will ensure the array object isn't collected.
*
- * Currently just using global references.
- *
- * TODO: if the same array gets pinned more than N times, print a warning
- * (but only if CheckJNI is enabled)
+ * We use a separate reference table, which is part of the GC root set.
*/
static void pinPrimitiveArray(ArrayObject* arrayObj)
{
- (void) addGlobalReference((Object*) arrayObj);
+ if (arrayObj == NULL)
+ return;
+
+ dvmLockMutex(&gDvm.jniPinRefLock);
+ if (!dvmAddToReferenceTable(&gDvm.jniPinRefTable, (Object*)arrayObj)) {
+ dvmDumpReferenceTable(&gDvm.jniPinRefTable, "JNI pinned array");
+ LOGE("Failed adding to JNI pinned array ref table (%d entries)\n",
+ (int) dvmReferenceTableEntries(&gDvm.jniPinRefTable));
+ dvmDumpThread(dvmThreadSelf(), false);
+ dvmAbort();
+ }
+
+ /*
+ * If we're watching global ref usage, also keep an eye on these.
+ *
+ * The total number of pinned primitive arrays should be pretty small.
+ * A single array should not be pinned more than once or twice; any
+ * more than that is a strong indicator that a Release function is
+ * not being called.
+ */
+ if (kTrackGrefUsage && gDvm.jniGrefLimit != 0) {
+ int count = 0;
+ Object** ppObj = gDvm.jniPinRefTable.table;
+ while (ppObj < gDvm.jniPinRefTable.nextEntry) {
+ if (*ppObj++ == (Object*) arrayObj)
+ count++;
+ }
+
+ if (count > kPinComplainThreshold) {
+ LOGW("JNI: pin count on array %p (%s) is now %d\n",
+ arrayObj, arrayObj->obj.clazz->descriptor, count);
+ /* keep going */
+ }
+ }
+
+ dvmUnlockMutex(&gDvm.jniPinRefLock);
}
/*
* Un-pin the array object. If an object was pinned twice, it must be
* unpinned twice before it's free to move.
- *
- * Currently just removing a global reference.
*/
static void unpinPrimitiveArray(ArrayObject* arrayObj)
{
- (void) deleteGlobalReference((jobject) arrayObj);
+ if (arrayObj == NULL)
+ return;
+
+ dvmLockMutex(&gDvm.jniPinRefLock);
+ if (!dvmRemoveFromReferenceTable(&gDvm.jniPinRefTable,
+ gDvm.jniPinRefTable.table, (Object*) arrayObj))
+ {
+ LOGW("JNI: unpinPrimitiveArray(%p) failed to find entry (valid=%d)\n",
+ arrayObj, dvmIsValidObject((Object*) arrayObj));
+ goto bail;
+ }
+
+bail:
+ dvmUnlockMutex(&gDvm.jniPinRefLock);
}
/*
* GC helper function to mark all JNI global references.
+ *
+ * We're currently handling the "pin" table here too.
*/
void dvmGcMarkJniGlobalRefs()
{
@@ -835,6 +888,16 @@
}
dvmUnlockMutex(&gDvm.jniGlobalRefLock);
+
+
+ dvmLockMutex(&gDvm.jniPinRefLock);
+
+ op = gDvm.jniPinRefTable.table;
+ while ((uintptr_t)op < (uintptr_t)gDvm.jniPinRefTable.nextEntry) {
+ dvmMarkObjectNonNull(*(op++));
+ }
+
+ dvmUnlockMutex(&gDvm.jniPinRefLock);
}
