Merge "fix a bug introduced when prepared-statement cache was made LRU-based"

core/java/android/database/sqlite/SQLiteCompiledSql.java

@@ -134,6 +134,10 @@ import android.util.Log;
         mInUse = false;
     }
 
+    /* package */ synchronized boolean isInUse() {
+        return mInUse;
+    }
+
     /**
      * Make sure that the native resource is cleaned up.
      */

core/java/android/database/sqlite/SQLiteDatabase.java

@@ -263,7 +263,7 @@ public class SQLiteDatabase extends SQLiteClosable {
      * invoked.
      *
      * this cache has an upper limit of mMaxSqlCacheSize (settable by calling the method
-     * (@link setMaxCacheSize(int)}).
+     * (@link setMaxSqlCacheSize(int)}).
      */
     // default statement-cache size per database connection ( = instance of this class)
     private int mMaxSqlCacheSize = 25;
@@ -271,7 +271,24 @@ public class SQLiteDatabase extends SQLiteClosable {
         new LinkedHashMap<String, SQLiteCompiledSql>(mMaxSqlCacheSize + 1, 0.75f, true) {
             @Override
             public boolean removeEldestEntry(Map.Entry<String, SQLiteCompiledSql> eldest) {
-                return this.size() > mMaxSqlCacheSize;
+                // eldest = least-recently used entry
+                // if it needs to be removed to accommodate a new entry,
+                //     close {@link SQLiteCompiledSql} represented by this entry, if not in use
+                //     and then let it be removed from the Map.
+                synchronized(mCompiledQueries) { // probably not necessary, but can't hurt
+                    if (this.size() <= mMaxSqlCacheSize) {
+                        // cache is not full. nothing needs to be removed
+                        return false;
+                    }
+                    // cache is full. eldest will be removed.
+                    SQLiteCompiledSql entry = eldest.getValue();
+                    if (!entry.isInUse()) {
+                        // this {@link SQLiteCompiledSql} is not in use. release it.
+                        entry.releaseSqlStatement();
+                    }
+                    // return true, so that this entry is removed automatically by the caller.
+                    return true;
+                }
             }
         };
     /**
@@ -2006,10 +2023,8 @@ public class SQLiteDatabase extends SQLiteClosable {
                         mCompiledQueries.size() + "|" + sql);
             }
         }
-        return;
     }
 
-
     private void deallocCachedSqlStatements() {
         synchronized (mCompiledQueries) {
             for (SQLiteCompiledSql compiledSql : mCompiledQueries.values()) {
@@ -2044,44 +2059,6 @@ public class SQLiteDatabase extends SQLiteClosable {
         return compiledStatement;
     }
 
-    /**
-     * returns true if the given sql is cached in compiled-sql cache.
-     * @hide
-     */
-    public boolean isInCompiledSqlCache(String sql) {
-        synchronized(mCompiledQueries) {
-            return mCompiledQueries.containsKey(sql);
-        }
-    }
-
-    /**
-     * purges the given sql from the compiled-sql cache.
-     * @hide
-     */
-    public void purgeFromCompiledSqlCache(String sql) {
-        synchronized(mCompiledQueries) {
-            mCompiledQueries.remove(sql);
-        }
-    }
-
-    /**
-     * remove everything from the compiled sql cache
-     * @hide
-     */
-    public void resetCompiledSqlCache() {
-        synchronized(mCompiledQueries) {
-            mCompiledQueries.clear();
-        }
-    }
-
-    /**
-     * return the current maxCacheSqlCacheSize
-     * @hide
-     */
-    public synchronized int getMaxSqlCacheSize() {
-        return mMaxSqlCacheSize;
-    }
-
     /**
      * set the max size of the prepared-statement cache for this database.
      * (size of the cache = number of compiled-sql-statements stored in the cache).

core/java/android/database/sqlite/SQLiteProgram.java

@@ -147,7 +147,7 @@ public abstract class SQLiteProgram extends SQLiteClosable {
      * @return a unique identifier for this program
      */
     public final int getUniqueId() {
-        return nStatement;
+        return (mCompiledSql != null) ? mCompiledSql.nStatement : 0;
     }
 
     /* package */ String getSqlString() {

core/tests/coretests/src/android/database/DatabaseGeneralTest.java

@@ -33,6 +33,7 @@ import android.util.Log;
 import junit.framework.Assert;
 
 import java.io.File;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Locale;
 
@@ -519,13 +520,13 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT
         assertEquals(1, c.getCount());
         assertTrue(c.moveToFirst());
         assertEquals("don't forget to handled 's", c.getString(1));
-        c.close();
+        c.deactivate();
 
         // make sure code should checking null string properly so that
         // it won't crash
         try {
             mDatabase.query("test", new String[]{"_id"},
-                    "_id=?", null, null, null, null);
+                    "_id=?", new String[]{null}, null, null, null);
             fail("expected exception not thrown");
         } catch (IllegalArgumentException e) {
             // expected
@@ -1136,5 +1137,44 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT
                 }
             }
         }
-    }    
+    }
+
+    @SmallTest
+    public void testLruCachingOfSqliteCompiledSqlObjs() {
+        mDatabase.execSQL("CREATE TABLE test (i int, j int);");
+        mDatabase.execSQL("insert into test values(1,1);");
+        // set cache size
+        int N = SQLiteDatabase.MAX_SQL_CACHE_SIZE;
+        mDatabase.setMaxSqlCacheSize(N);
+
+        // do N+1 queries - and when the 0th entry is removed from LRU cache due to the
+        // insertion of (N+1)th entry, make sure 0th entry is closed
+        ArrayList<SQLiteStatement> stmtObjs = new ArrayList<SQLiteStatement>();
+        for (int i = 0; i < N+1; i++) {
+            SQLiteStatement c = mDatabase.compileStatement("select * from test where i = " + i);
+            c.close();
+            stmtObjs.add(i, c);
+        }
+
+        assertEquals(0, stmtObjs.get(0).getUniqueId());
+        for (int i = 1; i < N+1; i++) {
+            assertTrue(stmtObjs.get(i).getUniqueId() > 0);
+        }
+    }
+
+    @SmallTest
+    public void testSetMaxCahesize() {
+        mDatabase.execSQL("CREATE TABLE test (i int, j int);");
+        mDatabase.execSQL("insert into test values(1,1);");
+        // set cache size
+        int N = SQLiteDatabase.MAX_SQL_CACHE_SIZE;
+        mDatabase.setMaxSqlCacheSize(N);
+
+        // try reduce cachesize
+        try {
+            mDatabase.setMaxSqlCacheSize(1);
+        } catch (IllegalStateException e) {
+            assertTrue(e.getMessage().contains("cannot set cacheSize to a value less than"));
+        }
+    }
 }