Bug 726821 - Push Cursor usage down into native bridge code. r=blassey
authorGian-Carlo Pascutto <gpascutto@mozilla.com>
Mon, 27 Feb 2012 12:28:22 +0100
changeset 87833 0bf80d3cebf5d8cdc251d2c4432651350eddc3c7
parent 87832 3e6935243310b57940c140238306dab8fefafa51
child 87834 bebeca2270ee23849c85b18707900dcf273fc3b3
push id22160
push usermbrubeck@mozilla.com
push dateTue, 28 Feb 2012 17:21:33 +0000
treeherdermozilla-central@dde4e0089a18 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersblassey
bugs726821
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 726821 - Push Cursor usage down into native bridge code. r=blassey
mobile/android/base/sqlite/SQLiteBridge.java
mozglue/android/SQLiteBridge.cpp
--- a/mobile/android/base/sqlite/SQLiteBridge.java
+++ b/mobile/android/base/sqlite/SQLiteBridge.java
@@ -25,30 +25,26 @@ import java.util.Set;
  * which might use whatever outdated DB is present on the Android system.
  */
 public class SQLiteBridge {
     private static final String LOGTAG = "SQLiteBridge";
 
     // Path to the database. We reopen it every query.
     private String mDb;
 
-    // Remember column names from last query result.
-    private ArrayList<String> mColumns;
-    private Long[] mQueryResults;
+    // Values remembered after a query.
+    private long[] mQueryResults;
 
-    // Values remembered after a query.
-    private int kResultInsertRowId = 0;
-    private int kResultRowsChanged = 1;
+    private static final int RESULT_INSERT_ROW_ID = 0;
+    private static final int RESULT_ROWS_CHANGED = 1;
 
     // JNI code in $(topdir)/mozglue/android/..
-    private static native void sqliteCall(String aDb, String aQuery,
-                                          String[] aParams,
-                                          ArrayList<String> aColumns,
-                                          Long[] aUpdateResult,
-                                          ArrayList<Object[]> aRes)
+    private static native MatrixBlobCursor sqliteCall(String aDb, String aQuery,
+                                                      String[] aParams,
+                                                      long[] aUpdateResult)
         throws SQLiteBridgeException;
 
     // Takes the path to the database we want to access.
     public SQLiteBridge(String aDb) throws SQLiteBridgeException {
         mDb = aDb;
     }
 
     // Executes a simple line of sql.
@@ -68,17 +64,17 @@ public class SQLiteBridge {
                throws SQLiteBridgeException {
         StringBuilder sb = new StringBuilder("DELETE from ");
         sb.append(table);
         if (whereClause != null) {
             sb.append(" WHERE " + whereClause);
         }
 
         internalQuery(sb.toString(), whereArgs);
-        return mQueryResults[kResultRowsChanged].intValue();
+        return (int)mQueryResults[RESULT_ROWS_CHANGED];
     }
 
     public Cursor query(String table,
                         String[] columns,
                         String selection,
                         String[] selectionArgs,
                         String groupBy,
                         String having,
@@ -114,32 +110,17 @@ public class SQLiteBridge {
             sb.append(" " + limit);
         }
 
         return rawQuery(sb.toString(), selectionArgs);
     }
 
     public Cursor rawQuery(String sql, String[] selectionArgs)
         throws SQLiteBridgeException {
-        ArrayList<Object[]> results;
-        results = internalQuery(sql, selectionArgs);
-
-        MatrixBlobCursor cursor =
-            new MatrixBlobCursor(mColumns.toArray(new String[0]));
-        try {
-            for (Object resultRow: results) {
-                Object[] resultColumns = (Object[])resultRow;
-                if (resultColumns.length == mColumns.size())
-                    cursor.addRow(resultColumns);
-            }
-        } catch(IllegalArgumentException ex) {
-            Log.e(LOGTAG, "Error getting rows", ex);
-        }
-
-        return cursor;
+        return internalQuery(sql, selectionArgs);
     }
 
     public long insert(String table, String nullColumnHack, ContentValues values)
                throws SQLiteBridgeException {
         Set<Entry<String, Object>> valueSet = values.valueSet();
         Iterator<Entry<String, Object>> valueIterator = valueSet.iterator();
         ArrayList<String> valueNames = new ArrayList<String>();
         ArrayList<String> valueBinds = new ArrayList<String>();
@@ -162,17 +143,17 @@ public class SQLiteBridge {
         // XXX - Do we need to bind these values?
         sb.append(" VALUES (");
         sb.append(TextUtils.join(", ", valueNames));
         sb.append(") ");
 
         String[] binds = new String[valueBinds.size()];
         valueBinds.toArray(binds);
         internalQuery(sb.toString(), binds);
-        return mQueryResults[kResultInsertRowId];
+        return mQueryResults[RESULT_INSERT_ROW_ID];
     }
 
     public int update(String table, ContentValues values, String whereClause, String[] whereArgs)
                throws SQLiteBridgeException {
         Set<Entry<String, Object>> valueSet = values.valueSet();
         Iterator<Entry<String, Object>> valueIterator = valueSet.iterator();
         ArrayList<String> valueNames = new ArrayList<String>();
 
@@ -197,47 +178,35 @@ public class SQLiteBridge {
                 valueNames.add(whereArgs[i]);
             }
         }
 
         String[] binds = new String[valueNames.size()];
         valueNames.toArray(binds);
 
         internalQuery(sb.toString(), binds);
-        return mQueryResults[kResultRowsChanged].intValue();
+        return (int)mQueryResults[RESULT_ROWS_CHANGED];
     }
 
     public int getVersion()
                throws SQLiteBridgeException {
-        ArrayList<Object[]> results = null;
-        results = internalQuery("PRAGMA user_version", null);
+        Cursor cursor = internalQuery("PRAGMA user_version", null);
         int ret = -1;
-        if (results != null) {
-            for (Object resultRow: results) {
-                Object[] resultColumns = (Object[])resultRow;
-                String version = (String)resultColumns[0];
-                ret = Integer.parseInt(version);
-            }
+        if (cursor != null) {
+            cursor.moveToFirst();
+            String version = cursor.getString(0);
+            ret = Integer.parseInt(version);
         }
         return ret;
     }
 
     // Do an SQL query, substituting the parameters in the query with the passed
     // parameters. The parameters are subsituded in order, so named parameters
     // are not supported.
-    // The result is returned as an ArrayList<Object[]>, with each
-    // row being an entry in the ArrayList, and each column being one Object
-    // in the Object[] array. The columns are of type null,
-    // direct ByteBuffer (BLOB), or String (everything else).
-    private ArrayList<Object[]> internalQuery(String aQuery, String[] aParams)
+    private Cursor internalQuery(String aQuery, String[] aParams)
         throws SQLiteBridgeException {
-        ArrayList<Object[]> result = new ArrayList<Object[]>();
-        mQueryResults = new Long[2];
-        mColumns = new ArrayList<String>();
-
-        sqliteCall(mDb, aQuery, aParams, mColumns, mQueryResults, result);
-
-        return result;
+        mQueryResults = new long[2];
+        return sqliteCall(mDb, aQuery, aParams, mQueryResults);
     }
 
     // nop, provided for API compatibility with SQLiteDatabase.
     public void close() { }
 }
--- a/mozglue/android/SQLiteBridge.cpp
+++ b/mozglue/android/SQLiteBridge.cpp
@@ -90,23 +90,21 @@ void setup_sqlite_functions(void *sqlite
   GETFUNC(sqlite3_changes);
   GETFUNC(sqlite3_last_insert_rowid);
 #undef GETFUNC
 }
 
 static bool initialized = false;
 static jclass stringClass;
 static jclass objectClass;
-static jclass longClass;
 static jclass byteBufferClass;
-static jclass arrayListClass;
+static jclass cursorClass;
 static jmethodID jByteBufferAllocateDirect;
-static jmethodID jArrayListAdd;
-static jmethodID jLongConstructor;
-static jobject jNull;
+static jmethodID jCursorConstructor;
+static jmethodID jCursorAddRow;
 
 static void
 JNI_Throw(JNIEnv* jenv, const char* name, const char* msg)
 {
     jclass cls = jenv->FindClass(name);
     if (cls == NULL) {
         LOG("Couldn't find exception class (or exception pending)\n");
         return;
@@ -118,65 +116,62 @@ JNI_Throw(JNIEnv* jenv, const char* name
     jenv->DeleteLocalRef(cls);
 }
 
 static void
 JNI_Setup(JNIEnv* jenv)
 {
     if (initialized) return;
 
-    objectClass     = jenv->FindClass("java/lang/Object");
-    stringClass     = jenv->FindClass("java/lang/String");
-    longClass       = jenv->FindClass("java/lang/Long");
-    byteBufferClass = jenv->FindClass("java/nio/ByteBuffer");
-    arrayListClass  = jenv->FindClass("java/util/ArrayList");
-    jNull           = jenv->NewGlobalRef(NULL);
+    objectClass       = jenv->FindClass("java/lang/Object");
+    stringClass       = jenv->FindClass("java/lang/String");
+    byteBufferClass   = jenv->FindClass("java/nio/ByteBuffer");
+    cursorClass       = jenv->FindClass("org/mozilla/gecko/sqlite/MatrixBlobCursor");
 
     if (stringClass == NULL || objectClass == NULL
-        || byteBufferClass == NULL || arrayListClass == NULL
-        || longClass == NULL) {
+        || byteBufferClass == NULL
+        || cursorClass == NULL) {
         LOG("Error finding classes");
         JNI_Throw(jenv, "org/mozilla/gecko/sqlite/SQLiteBridgeException",
                   "FindClass error");
         return;
     }
 
     // public static ByteBuffer allocateDirect(int capacity)
     jByteBufferAllocateDirect =
         jenv->GetStaticMethodID(byteBufferClass, "allocateDirect", "(I)Ljava/nio/ByteBuffer;");
-    // boolean add(Object o)
-    jArrayListAdd =
-        jenv->GetMethodID(arrayListClass, "add", "(Ljava/lang/Object;)Z");
-    // new Long(long i)
-    jLongConstructor =
-        jenv->GetMethodID(longClass, "<init>", "(J)V");
+    // new MatrixBlobCursor(String [])
+    jCursorConstructor =
+        jenv->GetMethodID(cursorClass, "<init>", "([Ljava/lang/String;)V");
+    // public void addRow (Object[] columnValues)
+    jCursorAddRow =
+        jenv->GetMethodID(cursorClass, "addRow", "([Ljava/lang/Object;)V");
 
     if (jByteBufferAllocateDirect == NULL
-        || jArrayListAdd == NULL
-        || jLongConstructor == NULL) {
+        || jCursorConstructor == NULL
+        || jCursorAddRow == NULL) {
         LOG("Error finding methods");
         JNI_Throw(jenv, "org/mozilla/gecko/sqlite/SQLiteBridgeException",
                   "GetMethodId error");
         return;
     }
 
     initialized = true;
 }
 
-extern "C" NS_EXPORT void JNICALL
+extern "C" NS_EXPORT jobject JNICALL
 Java_org_mozilla_gecko_sqlite_SQLiteBridge_sqliteCall(JNIEnv* jenv, jclass,
                                                       jstring jDb,
                                                       jstring jQuery,
                                                       jobjectArray jParams,
-                                                      jobject jColumns,
-                                                      jobjectArray jQueryRes,
-                                                      jobject jArrayList)
+                                                      jlongArray jQueryRes)
 {
     JNI_Setup(jenv);
 
+    jobject jCursor = NULL;
     char* errorMsg;
     jsize numPars = 0;
 
     const char* queryStr;
     queryStr = jenv->GetStringUTFChars(jQuery, NULL);
 
     const char* dbPath;
     dbPath = jenv->GetStringUTFChars(jDb, NULL);
@@ -239,37 +234,54 @@ Java_org_mozilla_gecko_sqlite_SQLiteBrid
 
     // Execute the query and step through the results
     rc = f_sqlite3_step(ppStmt);
     if (rc != SQLITE_ROW && rc != SQLITE_DONE) {
         asprintf(&errorMsg, "Can't step statement: (%d) %s\n", rc, f_sqlite3_errmsg(db));
         goto error_close;
     }
 
-    // Get the column names
+    // Get the column count and names
     int cols;
     cols = f_sqlite3_column_count(ppStmt);
-    for (int i = 0; i < cols; i++) {
-        const char* colName = f_sqlite3_column_name(ppStmt, i);
-        jstring jStr = jenv->NewStringUTF(colName);
-        jenv->CallBooleanMethod(jColumns, jArrayListAdd, jStr);
-        jenv->DeleteLocalRef(jStr);
+
+    {
+        // Allocate a String[cols]
+        jobjectArray jStringArray = jenv->NewObjectArray(cols,
+                                                         stringClass,
+                                                         NULL);
+        if (jStringArray == NULL) {
+            asprintf(&errorMsg, "Can't allocate String[]\n");
+            goto error_close;
+        }
+
+        // Assign column names to the String[]
+        for (int i = 0; i < cols; i++) {
+            const char* colName = f_sqlite3_column_name(ppStmt, i);
+            jstring jStr = jenv->NewStringUTF(colName);
+            jenv->SetObjectArrayElement(jStringArray, i, jStr);
+        }
+
+        // Construct the MatrixCursor(String[]) with given column names
+        jCursor = jenv->NewObject(cursorClass,
+                                  jCursorConstructor,
+                                  jStringArray);
+        if (jCursor == NULL) {
+            asprintf(&errorMsg, "Can't allocate MatrixBlobCursor\n");
+            goto error_close;
+        }
     }
 
     // Return the id and number of changed rows in jQueryRes
     {
-        long id = f_sqlite3_last_insert_rowid(db);
-        jobject jId = jenv->NewObject(longClass, jLongConstructor, id);
-        jenv->SetObjectArrayElement(jQueryRes, 0, jId);
-        jenv->DeleteLocalRef(jId);
+        jlong id = f_sqlite3_last_insert_rowid(db);
+        jenv->SetLongArrayRegion(jQueryRes, 0, 1, &id);
 
-        long changed = f_sqlite3_changes(db);
-        jobject jChanged = jenv->NewObject(longClass, jLongConstructor, changed);
-        jenv->SetObjectArrayElement(jQueryRes, 1, jChanged);
-        jenv->DeleteLocalRef(jChanged);
+        jlong changed = f_sqlite3_changes(db);
+        jenv->SetLongArrayRegion(jQueryRes, 1, 1, &changed);
     }
 
     // For each row, add an Object[] to the passed ArrayList,
     // with that containing either String or ByteArray objects
     // containing the columns
     while (rc != SQLITE_DONE) {
         // Process row
         // Construct Object[]
@@ -303,29 +315,28 @@ Java_org_mozilla_gecko_sqlite_SQLiteBrid
                     asprintf(&errorMsg, "Failure calling GetDirectBufferAddress\n");
                     goto error_close;
                 }
                 memcpy(bufferArray, blob, colLen);
 
                 jenv->SetObjectArrayElement(jRow, i, jByteBuffer);
                 jenv->DeleteLocalRef(jByteBuffer);
             } else if (colType == SQLITE_NULL) {
-                jenv->SetObjectArrayElement(jRow, i, jNull);
+                jenv->SetObjectArrayElement(jRow, i, NULL);
             } else {
                 // Treat everything else as text
                 const char* txt = (const char*)f_sqlite3_column_text(ppStmt, i);
                 jstring jStr = jenv->NewStringUTF(txt);
                 jenv->SetObjectArrayElement(jRow, i, jStr);
                 jenv->DeleteLocalRef(jStr);
             }
         }
 
-        // Append Object[] to ArrayList<Object[]>
-        // JNI doesn't know about the generic, so use Object[] as Object
-        jenv->CallBooleanMethod(jArrayList, jArrayListAdd, jRow);
+        // Append Object[] to Cursor
+        jenv->CallVoidMethod(jCursor, jCursorAddRow, jRow);
 
         // Clean up
         jenv->DeleteLocalRef(jRow);
 
         // Get next row
         rc = f_sqlite3_step(ppStmt);
         // Real error?
         if (rc != SQLITE_ROW && rc != SQLITE_DONE) {
@@ -336,17 +347,17 @@ Java_org_mozilla_gecko_sqlite_SQLiteBrid
 
     rc = f_sqlite3_finalize(ppStmt);
     if (rc != SQLITE_OK) {
         asprintf(&errorMsg, "Can't finalize statement: %s\n", f_sqlite3_errmsg(db));
         goto error_close;
     }
 
     f_sqlite3_close(db);
-    return;
+    return jCursor;
 
 error_close:
     f_sqlite3_close(db);
     LOG("Error in SQLiteBridge: %s\n", errorMsg);
     JNI_Throw(jenv, "org/mozilla/gecko/sqlite/SQLiteBridgeException", errorMsg);
     free(errorMsg);
-    return;
+    return jCursor;
 }