From c26bb63b50c7a855d25b396b1bf23a3aa6929b48 Mon Sep 17 00:00:00 2001
From: Andy McFadden <fadden@android.com>
Date: Fri, 21 Aug 2009 12:01:31 -0700
Subject: [PATCH] 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.)
---
 vm/Globals.h |  6 +++++
 vm/Jni.c     | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/vm/Globals.h b/vm/Globals.h
index 2ac118709..a1a1b830f 100644
--- a/vm/Globals.h
+++ b/vm/Globals.h
@@ -415,6 +415,12 @@ struct DvmGlobals {
     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 0a8103b89..4df32b1aa 100644
--- a/vm/Jni.c
+++ b/vm/Jni.c
@@ -349,9 +349,12 @@ DalvikJniReturnType dvmGetArgInfoReturnType(int jniArgInfo)
 #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 @@ bool dvmJniStartup(void)
         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 @@ bail:
  * 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 @@ void dvmGcMarkJniGlobalRefs()
     }
 
     dvmUnlockMutex(&gDvm.jniGlobalRefLock);
+
+
+    dvmLockMutex(&gDvm.jniPinRefLock);
+
+    op = gDvm.jniPinRefTable.table;
+    while ((uintptr_t)op < (uintptr_t)gDvm.jniPinRefTable.nextEntry) {
+        dvmMarkObjectNonNull(*(op++));
+    }
+
+    dvmUnlockMutex(&gDvm.jniPinRefLock);
 }
 
 
-- 
2.11.0