fix a bug in compiled-sql caching & hide public api setMaxSqlCacheSize

this is a clone of https://android-git.corp.google.com/g/#change,35174.
if the cache is full to its capacity and if a new statement is to be cached,
one of the entries in the cache is thrown out to make room for the new one.
but the one that is thrown out doesn't get deallocated by SQLiteProgram
because it doesn't know that it should.
fixed this by having SQLiteProgram finalize its sql statement in
releaseReference*() methods, if the statement is not in cache.

2 files changed, 158 insertions, 95 deletions

diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java
index f31058661331..b59030d85a77 100644
--- a/core/java/android/database/sqlite/SQLiteDatabase.java
+++ b/core/java/android/database/sqlite/SQLiteDatabase.java
@@ -243,9 +243,12 @@ public class SQLiteDatabase extends SQLiteClosable {
      * (@link setMaxCacheSize(int)}). its default is 0 - i.e., no caching by default because
      * most of the apps don't use "?" syntax in their sql, caching is not useful for them.
      */
-    private Map<String, SQLiteCompiledSql> mCompiledQueries = Maps.newHashMap();
-    private int mMaxSqlCacheSize = 0; // no caching by default
-    private static final int MAX_SQL_CACHE_SIZE = 1000;
+    /* package */ Map<String, SQLiteCompiledSql> mCompiledQueries = Maps.newHashMap();
+    /**
+     * @hide
+     */
+    public static final int MAX_SQL_CACHE_SIZE = 250;
+    private int mMaxSqlCacheSize = MAX_SQL_CACHE_SIZE; // max cache size per Database instance
 
     /** maintain stats about number of cache hits and misses */
     private int mNumCacheHits;
@@ -828,6 +831,15 @@ public class SQLiteDatabase extends SQLiteClosable {
     }
 
     private void closeClosable() {
+        /* deallocate all compiled sql statement objects from mCompiledQueries cache.
+         * this should be done before de-referencing all {@link SQLiteClosable} objects
+         * from this database object because calling
+         * {@link SQLiteClosable#onAllReferencesReleasedFromContainer()} could cause the database
+         * to be closed. sqlite doesn't let a database close if there are
+         * any unfinalized statements - such as the compiled-sql objects in mCompiledQueries.
+         */
+        deallocCachedSqlStatements();
+
         Iterator<Map.Entry<SQLiteClosable, Object>> iter = mPrograms.entrySet().iterator();
         while (iter.hasNext()) {
             Map.Entry<SQLiteClosable, Object> entry = iter.next();
@@ -836,13 +848,6 @@ public class SQLiteDatabase extends SQLiteClosable {
                 program.onAllReferencesReleasedFromContainer();
             }
         }
-
-        // finalize all compiled sql statement objects in compiledQueries cache
-        synchronized (mCompiledQueries) {
-            for (SQLiteCompiledSql compiledStatement : mCompiledQueries.values()) {
-                compiledStatement.releaseSqlStatement();
-            }
-        }
     }
 
     /**
@@ -1781,30 +1786,61 @@ public class SQLiteDatabase extends SQLiteClosable {
         return mPath;
     }
 
-    /**
-     * set the max size of the compiled sql cache for this database after purging the cache.
-     * (size of the cache = number of compiled-sql-statements stored in the cache)
-     *
-     * synchronized because we don't want t threads to change cache size at the same time.
-     * @param cacheSize the size of the cache. can be (0 to MAX_SQL_CACHE_SIZE)
-     */
-    public void setMaxSqlCacheSize(int cacheSize) {
-        synchronized(mCompiledQueries) {
-            resetCompiledSqlCache();
-            mMaxSqlCacheSize = (cacheSize > MAX_SQL_CACHE_SIZE) ? MAX_SQL_CACHE_SIZE
-                    : (cacheSize < 0) ? 0 : cacheSize;
+
+
+    /* package */ void logTimeStat(String sql, long beginNanos) {
+        // Sample fast queries in proportion to the time taken.
+        // Quantize the % first, so the logged sampling probability
+        // exactly equals the actual sampling rate for this query.
+
+        int samplePercent;
+        long nanos = Debug.threadCpuTimeNanos() - beginNanos;
+        if (nanos >= QUERY_LOG_TIME_IN_NANOS) {
+            samplePercent = 100;
+        } else {
+            samplePercent = (int) (100 * nanos / QUERY_LOG_TIME_IN_NANOS) + 1;
+            if (mRandom.nextInt(100) >= samplePercent) return;
         }
+
+        if (sql.length() > QUERY_LOG_SQL_LENGTH) sql = sql.substring(0, QUERY_LOG_SQL_LENGTH);
+
+        // ActivityThread.currentPackageName() only returns non-null if the
+        // current thread is an application main thread.  This parameter tells
+        // us whether an event loop is blocked, and if so, which app it is.
+        //
+        // Sadly, there's no fast way to determine app name if this is *not* a
+        // main thread, or when we are invoked via Binder (e.g. ContentProvider).
+        // Hopefully the full path to the database will be informative enough.
+
+        String blockingPackage = ActivityThread.currentPackageName();
+        if (blockingPackage == null) blockingPackage = "";
+
+        int millis = (int) (nanos / 1000000);
+        EventLog.writeEvent(EVENT_DB_OPERATION, mPath, sql, millis, blockingPackage, samplePercent);
     }
 
     /**
-     * remove everything from the compiled sql cache
+     * Sets the locale for this database.  Does nothing if this database has
+     * the NO_LOCALIZED_COLLATORS flag set or was opened read only.
+     * @throws SQLException if the locale could not be set.  The most common reason
+     * for this is that there is no collator available for the locale you requested.
+     * In this case the database remains unchanged.
      */
-    public void resetCompiledSqlCache() {
-        synchronized(mCompiledQueries) {
-            mCompiledQueries.clear();
+    public void setLocale(Locale locale) {
+        lock();
+        try {
+            native_setLocale(locale.toString(), mFlags);
+        } finally {
+            unlock();
         }
     }
 
+    /*
+     * ============================================================================
+     *
+     *       The following methods deal with compiled-sql cache
+     * ============================================================================
+     */
     /**
      * adds the given sql and its compiled-statement-id-returned-by-sqlite to the
      * cache of compiledQueries attached to 'this'.
@@ -1812,16 +1848,14 @@ public class SQLiteDatabase extends SQLiteClosable {
      * if there is already a {@link SQLiteCompiledSql} in compiledQueries for the given sql,
      * the new {@link SQLiteCompiledSql} object is NOT inserted into the cache (i.e.,the current
      * mapping is NOT replaced with the new mapping).
-     *
-     * @return true if the given obj is added to cache. false otherwise.
      */
-    /* package */ boolean addToCompiledQueries(String sql, SQLiteCompiledSql compiledStatement) {
+    /* package */ void addToCompiledQueries(String sql, SQLiteCompiledSql compiledStatement) {
         if (mMaxSqlCacheSize == 0) {
             // for this database, there is no cache of compiled sql.
             if (SQLiteDebug.DEBUG_SQL_CACHE) {
                 Log.v(TAG, "|NOT adding_sql_to_cache|" + getPath() + "|" + sql);
             }
-            return false;
+            return;
         }
 
         SQLiteCompiledSql compiledSql = null;
@@ -1829,30 +1863,42 @@ public class SQLiteDatabase extends SQLiteClosable {
             // don't insert the new mapping if a mapping already exists
             compiledSql = mCompiledQueries.get(sql);
             if (compiledSql != null) {
-                return false;
+                return;
             }
             // add this <sql, compiledStatement> to the cache
             if (mCompiledQueries.size() == mMaxSqlCacheSize) {
                 /* reached max cachesize. before adding new entry, remove an entry from the
                  * cache. we don't want to wipe out the entire cache because of this:
                  * GCing {@link SQLiteCompiledSql} requires call to sqlite3_finalize
-                 * JNI method. If entire cache is wiped out, it could be cause a big GC activity
+                 * JNI method. If entire cache is wiped out, it could cause a big GC activity
                  * just because a (rogue) process is using the cache incorrectly.
                  */
+                Log.wtf(TAG, "Too many sql statements in database cache. Make sure your sql " +
+                        "statements are using prepared-sql-statement syntax with '?' for" +
+                        "bindargs, instead of using actual values");
                 Set<String> keySet = mCompiledQueries.keySet();
                 for (String s : keySet) {
                     mCompiledQueries.remove(s);
                     break;
                 }
             }
-            compiledSql = new SQLiteCompiledSql(this, sql);
-            mCompiledQueries.put(sql, compiledSql);
+            mCompiledQueries.put(sql, compiledStatement);
         }
         if (SQLiteDebug.DEBUG_SQL_CACHE) {
             Log.v(TAG, "|adding_sql_to_cache|" + getPath() + "|" + mCompiledQueries.size() + "|" +
                     sql);
         }
-        return true;
+        return;
+    }
+
+
+    private void deallocCachedSqlStatements() {
+        synchronized (mCompiledQueries) {
+            for (SQLiteCompiledSql compiledSql : mCompiledQueries.values()) {
+                compiledSql.releaseSqlStatement();
+            }
+            mCompiledQueries.clear();
+        }
     }
 
     /**
@@ -1887,51 +1933,67 @@ public class SQLiteDatabase extends SQLiteClosable {
         return compiledStatement;
     }
 
-    /* package */ void logTimeStat(String sql, long beginNanos) {
-        // Sample fast queries in proportion to the time taken.
-        // Quantize the % first, so the logged sampling probability
-        // exactly equals the actual sampling rate for this query.
-
-        int samplePercent;
-        long nanos = Debug.threadCpuTimeNanos() - beginNanos;
-        if (nanos >= QUERY_LOG_TIME_IN_NANOS) {
-            samplePercent = 100;
-        } else {
-            samplePercent = (int) (100 * nanos / QUERY_LOG_TIME_IN_NANOS) + 1;
-            if (mRandom.nextInt(100) >= samplePercent) return;
+    /**
+     * returns true if the given sql is cached in compiled-sql cache.
+     * @hide
+     */
+    public boolean isInCompiledSqlCache(String sql) {
+        synchronized(mCompiledQueries) {
+            return mCompiledQueries.containsKey(sql);
         }
+    }
 
-        if (sql.length() > QUERY_LOG_SQL_LENGTH) sql = sql.substring(0, QUERY_LOG_SQL_LENGTH);
-
-        // ActivityThread.currentPackageName() only returns non-null if the
-        // current thread is an application main thread.  This parameter tells
-        // us whether an event loop is blocked, and if so, which app it is.
-        //
-        // Sadly, there's no fast way to determine app name if this is *not* a
-        // main thread, or when we are invoked via Binder (e.g. ContentProvider).
-        // Hopefully the full path to the database will be informative enough.
+    /**
+     * purges the given sql from the compiled-sql cache.
+     * @hide
+     */
+    public void purgeFromCompiledSqlCache(String sql) {
+        synchronized(mCompiledQueries) {
+            mCompiledQueries.remove(sql);
+        }
+    }
 
-        String blockingPackage = ActivityThread.currentPackageName();
-        if (blockingPackage == null) blockingPackage = "";
+    /**
+     * remove everything from the compiled sql cache
+     * @hide
+     */
+    public void resetCompiledSqlCache() {
+        synchronized(mCompiledQueries) {
+            mCompiledQueries.clear();
+        }
+    }
 
-        int millis = (int) (nanos / 1000000);
-        EventLog.writeEvent(EVENT_DB_OPERATION, mPath, sql, millis, blockingPackage, samplePercent);
+    /**
+     * return the current maxCacheSqlCacheSize
+     * @hide
+     */
+    public synchronized int getMaxSqlCacheSize() {
+        return mMaxSqlCacheSize;
     }
 
     /**
-     * Sets the locale for this database.  Does nothing if this database has
-     * the NO_LOCALIZED_COLLATORS flag set or was opened read only.
-     * @throws SQLException if the locale could not be set.  The most common reason
-     * for this is that there is no collator available for the locale you requested.
-     * In this case the database remains unchanged.
+     * set the max size of the compiled sql cache for this database after purging the cache.
+     * (size of the cache = number of compiled-sql-statements stored in the cache).
+     *
+     * max cache size can ONLY be increased from its current size (default = 0).
+     * if this method is called with smaller size than the current value of mMaxSqlCacheSize,
+     * then IllegalStateException is thrown
+     *
+     * synchronized because we don't want t threads to change cache size at the same time.
+     * @param cacheSize the size of the cache. can be (0 to MAX_SQL_CACHE_SIZE)
+     * @throws IllegalStateException if input cacheSize > MAX_SQL_CACHE_SIZE or < 0 or
+     * < the value set with previous setMaxSqlCacheSize() call.
+     *
+     * @hide
      */
-    public void setLocale(Locale locale) {
-        lock();
-        try {
-            native_setLocale(locale.toString(), mFlags);
-        } finally {
-            unlock();
+    public synchronized void setMaxSqlCacheSize(int cacheSize) {
+        if (cacheSize > MAX_SQL_CACHE_SIZE || cacheSize < 0) {
+            throw new IllegalStateException("expected value between 0 and " + MAX_SQL_CACHE_SIZE);
+        } else if (cacheSize < mMaxSqlCacheSize) {
+            throw new IllegalStateException("cannot set cacheSize to a value less than the value " +
+                    "set with previous setMaxSqlCacheSize() call.");
         }
+        mMaxSqlCacheSize = cacheSize;
     }
 
     /**
diff --git a/core/java/android/database/sqlite/SQLiteProgram.java b/core/java/android/database/sqlite/SQLiteProgram.java
index edc15cb0d824..00b0a8675ec0 100644
--- a/core/java/android/database/sqlite/SQLiteProgram.java
+++ b/core/java/android/database/sqlite/SQLiteProgram.java
@@ -37,15 +37,13 @@ public abstract class SQLiteProgram extends SQLiteClosable {
     protected int nHandle = 0;
 
     /**
-     * the compiledSql object for the given sql statement.
+     * the SQLiteCompiledSql object for the given sql statement.
      */
-    private SQLiteCompiledSql compiledSql;
-    private boolean myCompiledSqlIsInCache;
+    private SQLiteCompiledSql mCompiledSql;
 
     /**
-     * compiledSql statement id is populated with the corresponding object from the above
-     * member compiledSql.
-     * this member is used by the native_bind_* methods
+     * SQLiteCompiledSql statement id is populated with the corresponding object from the above
+     * member. This member is used by the native_bind_* methods
      */
     protected int nStatement = 0;
 
@@ -60,47 +58,50 @@ public abstract class SQLiteProgram extends SQLiteClosable {
         db.addSQLiteClosable(this);
         this.nHandle = db.mNativeHandle;
 
-        compiledSql = db.getCompiledStatementForSql(sql);
-        if (compiledSql == null) {
+        mCompiledSql = db.getCompiledStatementForSql(sql);
+        if (mCompiledSql == null) {
             // create a new compiled-sql obj
-            compiledSql = new SQLiteCompiledSql(db, sql);
+            mCompiledSql = new SQLiteCompiledSql(db, sql);
 
             // add it to the cache of compiled-sqls
-            myCompiledSqlIsInCache = db.addToCompiledQueries(sql, compiledSql);
-        } else {
-            myCompiledSqlIsInCache = true;
+            db.addToCompiledQueries(sql, mCompiledSql);
         }
-        nStatement = compiledSql.nStatement;
+        nStatement = mCompiledSql.nStatement;
     }
 
     @Override
     protected void onAllReferencesReleased() {
-        // release the compiled sql statement used by me if it is NOT in cache
-        if (!myCompiledSqlIsInCache && compiledSql != null) {
-            compiledSql.releaseSqlStatement();
-            compiledSql = null; // so that GC doesn't call finalize() on it
-        }
+        releaseCompiledSqlIfInCache();
         mDatabase.releaseReference();
         mDatabase.removeSQLiteClosable(this);
     }
 
     @Override
     protected void onAllReferencesReleasedFromContainer() {
-        // release the compiled sql statement used by me if it is NOT in cache
-      if (!myCompiledSqlIsInCache && compiledSql != null) {
-            compiledSql.releaseSqlStatement();
-            compiledSql = null; // so that GC doesn't call finalize() on it
-        }
+        releaseCompiledSqlIfInCache();
         mDatabase.releaseReference();
     }
 
+    private void releaseCompiledSqlIfInCache() {
+        if (mCompiledSql == null) {
+            return;
+        }
+        synchronized(mDatabase.mCompiledQueries) {
+            if (!mDatabase.mCompiledQueries.containsValue(mCompiledSql)) {
+                mCompiledSql.releaseSqlStatement();
+                mCompiledSql = null; // so that GC doesn't call finalize() on it
+                nStatement = 0;
+            }
+        }
+    }
+
     /**
      * Returns a unique identifier for this program.
      *
      * @return a unique identifier for this program
      */
     public final int getUniqueId() {
-        return compiledSql.nStatement;
+        return nStatement;
     }
 
     /* package */ String getSqlString() {