Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9504c70f authored by Vasu Nori's avatar Vasu Nori
Browse files

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

1. when an entry is pushed out of cache, it should be released if it is not
in use by any thread. This didn't have to be done when cache was NOT LRU
because the object either got into the cache while the caller had a reference
to it or it didn't. if it didn't get into cache (because cache is full),
the caller's close() released the object. But in LRU cache, an object
could get pushed out of cache due to LRU policy and if it is NOT in use
by any thread at the time it was pushed out of cache, then it
got GC'ed - which caused warnings to be pronted in the log.
2. also delete some unused methods in SQLiteDatabase.

Change-Id: I7831952647d3a057342bcc8ac186a6a26eb58f33
parent 9c7686b9

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

+4 −0
Original line number Diff line number Diff line
@@ -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

+19 −42
Original line number Diff line number Diff line
@@ -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

+1 −1
Original line number Diff line number Diff line
@@ -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

+43 −3
Original line number Diff line number Diff line
@@ -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
@@ -1137,4 +1138,43 @@ 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"));

        }

    }

}