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.
diff --git a/api/current.xml b/api/current.xml
index 96e7fd6..1142088 100644
--- a/api/current.xml
+++ b/api/current.xml
@@ -51735,17 +51735,6 @@
<exception name="SQLException" type="android.database.SQLException">
</exception>
</method>
-<method name="resetCompiledSqlCache"
- return="void"
- abstract="false"
- native="false"
- synchronized="false"
- static="false"
- final="false"
- deprecated="not deprecated"
- visibility="public"
->
-</method>
<method name="setLocale"
return="void"
abstract="false"
@@ -51772,19 +51761,6 @@
<parameter name="lockingEnabled" type="boolean">
</parameter>
</method>
-<method name="setMaxSqlCacheSize"
- return="void"
- abstract="false"
- native="false"
- synchronized="false"
- static="false"
- final="false"
- deprecated="not deprecated"
- visibility="public"
->
-<parameter name="cacheSize" type="int">
-</parameter>
-</method>
<method name="setMaximumSize"
return="long"
abstract="false"
diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java
index f310586..b59030d8 100644
--- a/core/java/android/database/sqlite/SQLiteDatabase.java
+++ b/core/java/android/database/sqlite/SQLiteDatabase.java
@@ -243,9 +243,12 @@
* (@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 @@
}
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 @@
program.onAllReferencesReleasedFromContainer();
}
}
-
- // finalize all compiled sql statement objects in compiledQueries cache
- synchronized (mCompiledQueries) {
- for (SQLiteCompiledSql compiledStatement : mCompiledQueries.values()) {
- compiledStatement.releaseSqlStatement();
- }
- }
}
/**
@@ -1781,111 +1786,7 @@
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;
- }
- }
- /**
- * remove everything from the compiled sql cache
- */
- public void resetCompiledSqlCache() {
- synchronized(mCompiledQueries) {
- mCompiledQueries.clear();
- }
- }
-
- /**
- * adds the given sql and its compiled-statement-id-returned-by-sqlite to the
- * cache of compiledQueries attached to 'this'.
- *
- * 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) {
- 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;
- }
-
- SQLiteCompiledSql compiledSql = null;
- synchronized(mCompiledQueries) {
- // don't insert the new mapping if a mapping already exists
- compiledSql = mCompiledQueries.get(sql);
- if (compiledSql != null) {
- return false;
- }
- // 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
- * just because a (rogue) process is using the cache incorrectly.
- */
- Set<String> keySet = mCompiledQueries.keySet();
- for (String s : keySet) {
- mCompiledQueries.remove(s);
- break;
- }
- }
- compiledSql = new SQLiteCompiledSql(this, sql);
- mCompiledQueries.put(sql, compiledSql);
- }
- if (SQLiteDebug.DEBUG_SQL_CACHE) {
- Log.v(TAG, "|adding_sql_to_cache|" + getPath() + "|" + mCompiledQueries.size() + "|" +
- sql);
- }
- return true;
- }
-
- /**
- * from the compiledQueries cache, returns the compiled-statement-id for the given sql.
- * returns null, if not found in the cache.
- */
- /* package */ SQLiteCompiledSql getCompiledStatementForSql(String sql) {
- SQLiteCompiledSql compiledStatement = null;
- boolean cacheHit;
- synchronized(mCompiledQueries) {
- if (mMaxSqlCacheSize == 0) {
- // for this database, there is no cache of compiled sql.
- if (SQLiteDebug.DEBUG_SQL_CACHE) {
- Log.v(TAG, "|cache NOT found|" + getPath());
- }
- return null;
- }
- cacheHit = (compiledStatement = mCompiledQueries.get(sql)) != null;
- }
- if (cacheHit) {
- mNumCacheHits++;
- } else {
- mNumCacheMisses++;
- }
-
- if (SQLiteDebug.DEBUG_SQL_CACHE) {
- Log.v(TAG, "|cache_stats|" +
- getPath() + "|" + mCompiledQueries.size() +
- "|" + mNumCacheHits + "|" + mNumCacheMisses +
- "|" + cacheHit + "|" + mTimeOpened + "|" + mTimeClosed + "|" + sql);
- }
- return compiledStatement;
- }
/* package */ void logTimeStat(String sql, long beginNanos) {
// Sample fast queries in proportion to the time taken.
@@ -1934,6 +1835,167 @@
}
}
+ /*
+ * ============================================================================
+ *
+ * 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'.
+ *
+ * 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).
+ */
+ /* 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;
+ }
+
+ SQLiteCompiledSql compiledSql = null;
+ synchronized(mCompiledQueries) {
+ // don't insert the new mapping if a mapping already exists
+ compiledSql = mCompiledQueries.get(sql);
+ if (compiledSql != null) {
+ 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 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;
+ }
+ }
+ mCompiledQueries.put(sql, compiledStatement);
+ }
+ if (SQLiteDebug.DEBUG_SQL_CACHE) {
+ Log.v(TAG, "|adding_sql_to_cache|" + getPath() + "|" + mCompiledQueries.size() + "|" +
+ sql);
+ }
+ return;
+ }
+
+
+ private void deallocCachedSqlStatements() {
+ synchronized (mCompiledQueries) {
+ for (SQLiteCompiledSql compiledSql : mCompiledQueries.values()) {
+ compiledSql.releaseSqlStatement();
+ }
+ mCompiledQueries.clear();
+ }
+ }
+
+ /**
+ * from the compiledQueries cache, returns the compiled-statement-id for the given sql.
+ * returns null, if not found in the cache.
+ */
+ /* package */ SQLiteCompiledSql getCompiledStatementForSql(String sql) {
+ SQLiteCompiledSql compiledStatement = null;
+ boolean cacheHit;
+ synchronized(mCompiledQueries) {
+ if (mMaxSqlCacheSize == 0) {
+ // for this database, there is no cache of compiled sql.
+ if (SQLiteDebug.DEBUG_SQL_CACHE) {
+ Log.v(TAG, "|cache NOT found|" + getPath());
+ }
+ return null;
+ }
+ cacheHit = (compiledStatement = mCompiledQueries.get(sql)) != null;
+ }
+ if (cacheHit) {
+ mNumCacheHits++;
+ } else {
+ mNumCacheMisses++;
+ }
+
+ if (SQLiteDebug.DEBUG_SQL_CACHE) {
+ Log.v(TAG, "|cache_stats|" +
+ getPath() + "|" + mCompiledQueries.size() +
+ "|" + mNumCacheHits + "|" + mNumCacheMisses +
+ "|" + cacheHit + "|" + mTimeOpened + "|" + mTimeClosed + "|" + sql);
+ }
+ 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 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 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;
+ }
+
/**
* Native call to open the database.
*
diff --git a/core/java/android/database/sqlite/SQLiteProgram.java b/core/java/android/database/sqlite/SQLiteProgram.java
index edc15cb..00b0a86 100644
--- a/core/java/android/database/sqlite/SQLiteProgram.java
+++ b/core/java/android/database/sqlite/SQLiteProgram.java
@@ -37,15 +37,13 @@
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 @@
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() {
diff --git a/tests/AndroidTests/src/com/android/unit_tests/DatabaseGeneralTest.java b/tests/AndroidTests/src/com/android/unit_tests/DatabaseGeneralTest.java
index 7a4d934..69d55c1 100644
--- a/tests/AndroidTests/src/com/android/unit_tests/DatabaseGeneralTest.java
+++ b/tests/AndroidTests/src/com/android/unit_tests/DatabaseGeneralTest.java
@@ -987,4 +987,18 @@
ih.close();
}
+ @MediumTest
+ public void testDbCloseReleasingAllCachedSql() {
+ mDatabase.execSQL("CREATE TABLE test (_id INTEGER PRIMARY KEY, text1 TEXT, text2 TEXT, " +
+ "num1 INTEGER, num2 INTEGER, image BLOB);");
+ final String statement = "DELETE FROM test WHERE _id=?;";
+ SQLiteStatement statementDoNotClose = mDatabase.compileStatement(statement);
+ assertTrue(statementDoNotClose.getUniqueId() > 0);
+ int nStatement = statementDoNotClose.getUniqueId();
+ assertTrue(statementDoNotClose.getUniqueId() == nStatement);
+ /* do not close statementDoNotClose object.
+ * That should leave it in SQLiteDatabase.mPrograms.
+ * mDatabase.close() in tearDown() should release it.
+ */
+ }
}
