From ab248525e0731b016fa0cce564c211d3b7ce0b3f Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Sun, 24 Apr 2016 20:09:14 +1000 Subject: [PATCH 1/4] Don't do a subquery in a subquery in a subquery. This was there as a workaround for #1, but that has subsequently been fixed. Thus, the hack is no longer required. Also removed an additional `AND` because it is already performed in the `JOIN`. I supsect this last one would've been eliminated by the sqlite optimizer anyway, but the query is slightly simpler now. This fix doesn't improve performance as much as I'd hoped, but it is something. --- .../java/org/fdroid/fdroid/data/AppProvider.java | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java index 96475ea42..f67803ecf 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -1057,20 +1057,7 @@ public class AppProvider extends FDroidProvider { " JOIN " + repo + " ON (" + repo + "._id = " + apk + ".repo) " + " WHERE " + app + ".id = " + apk + ".id AND " + - apk + ".vercode = ( " + - - // We only want the latest apk here. Ideally, we should - // instead join onto apk.suggestedVercode, but as per - // https://gitlab.com/fdroid/fdroidclient/issues/1 there - // may be some situations where suggestedVercode isn't - // set. - // TODO: If we can guarantee that suggestedVercode is set, - // then join onto that instead. This will save from doing - // a futher sub query for each app. - " SELECT MAX(inner_apk.vercode) " + - " FROM " + apk + " as inner_apk " + - " WHERE inner_apk.id = " + apk + ".id ) " + - " AND " + apk + ".repo = fdroid_repo._id "; + apk + ".vercode = " + app + ".suggestedVercode "; return " UPDATE " + app + " SET " + From 721dcb00c13e9c8dd6e104102aea380b46663153 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 28 Apr 2016 09:00:41 +1000 Subject: [PATCH 2/4] Use temporary, in memory database for update process. This increases the speed of the complex queries required at the end of the update process to: * calculate suggested version codes * figure out icon urls * etc, by two orders of magnitude. --- .../org/fdroid/fdroid/data/RepoPersister.java | 3 +- .../fdroid/fdroid/data/TempApkProvider.java | 18 ++++--- .../fdroid/fdroid/data/TempAppProvider.java | 50 ++++++++++++++----- 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java index 2138f8575..8f5d2e037 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java @@ -91,9 +91,8 @@ public class RepoPersister { // end of the process. This is due to the fact that we can't verify the cert // the index was signed with until we've finished reading it - and we don't // want to put stuff in the real database until we are sure it is from a - // trusted source. + // trusted source. It also helps performance as it is done via an in-memory database. TempAppProvider.Helper.init(context); - TempApkProvider.Helper.init(context); hasBeenInitialized = true; } diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java index 831b231ba..19b4d277d 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java @@ -68,12 +68,16 @@ public class TempApkProvider extends ApkProvider { /** * Deletes the old temporary table (if it exists). Then creates a new temporary apk provider * table and populates it with all the data from the real apk provider table. + * + * This is package local because it must be invoked after + * {@link org.fdroid.fdroid.data.TempAppProvider.Helper#init(Context)}. Due to this + * dependence, that method invokes this one itself, rather than leaving it to the + * {@link RepoPersister}. */ - public static void init(Context context) { + static void init(Context context) { Uri uri = Uri.withAppendedPath(getContentUri(), PATH_INIT); context.getContentResolver().insert(uri, new ContentValues()); } - } @Override @@ -123,11 +127,11 @@ public class TempApkProvider extends ApkProvider { private void initTable() { final SQLiteDatabase db = db(); - db.execSQL("DROP TABLE IF EXISTS " + getTableName()); - db.execSQL("CREATE TABLE " + getTableName() + " AS SELECT * FROM " + DBHelper.TABLE_APK); - db.execSQL("CREATE INDEX IF NOT EXISTS apk_vercode on " + getTableName() + " (vercode);"); - db.execSQL("CREATE INDEX IF NOT EXISTS apk_id on " + getTableName() + " (id);"); - db.execSQL("CREATE INDEX IF NOT EXISTS apk_compatible ON " + getTableName() + " (compatible);"); + final String memoryDbName = TempAppProvider.DB; + db.execSQL("CREATE TABLE " + memoryDbName + "." + getTableName() + " AS SELECT * FROM main." + DBHelper.TABLE_APK); + db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_vercode on " + getTableName() + " (vercode);"); + db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_id on " + getTableName() + " (id);"); + db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (compatible);"); } } diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java index 81a0737ff..86ac6bc59 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java @@ -3,8 +3,10 @@ package org.fdroid.fdroid.data; import android.content.ContentValues; import android.content.Context; import android.content.UriMatcher; +import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; import android.net.Uri; +import android.text.TextUtils; import android.util.Log; import org.fdroid.fdroid.Utils; @@ -16,6 +18,11 @@ public class TempAppProvider extends AppProvider { private static final String TAG = "TempAppProvider"; + /** + * The name of the in memory database used for updating. + */ + static final String DB = "temp_update_db"; + private static final String PROVIDER_NAME = "TempAppProvider"; private static final String TABLE_TEMP_APP = "temp_" + DBHelper.TABLE_APP; @@ -60,6 +67,7 @@ public class TempAppProvider extends AppProvider { public static void init(Context context) { Uri uri = Uri.withAppendedPath(getContentUri(), PATH_INIT); context.getContentResolver().insert(uri, new ContentValues()); + TempApkProvider.Helper.init(context); } /** @@ -111,13 +119,30 @@ public class TempAppProvider extends AppProvider { return count; } + private void ensureTempTableDetached(SQLiteDatabase db) { + Cursor cursor = db.rawQuery("PRAGMA database_list", null); + try { + cursor.moveToFirst(); + while (!cursor.isAfterLast()) { + String name = cursor.getString(cursor.getColumnIndex("name")); + if (TextUtils.equals(name, DB)) { + db.execSQL("DETACH DATABASE " + DB); + } + cursor.moveToNext(); + } + } finally { + cursor.close(); + } + } + private void initTable() { final SQLiteDatabase db = db(); - db.execSQL("DROP TABLE IF EXISTS " + getTableName()); - db.execSQL("CREATE TABLE " + getTableName() + " AS SELECT * FROM " + DBHelper.TABLE_APP); - db.execSQL("CREATE INDEX IF NOT EXISTS app_id ON " + getTableName() + " (id);"); - db.execSQL("CREATE INDEX IF NOT EXISTS app_upstreamVercode ON " + getTableName() + " (upstreamVercode);"); - db.execSQL("CREATE INDEX IF NOT EXISTS app_compatible ON " + getTableName() + " (compatible);"); + ensureTempTableDetached(db); + db.execSQL("ATTACH DATABASE ':memory:' AS " + DB); + db.execSQL("CREATE TABLE " + DB + "." + getTableName() + " AS SELECT * FROM main." + DBHelper.TABLE_APP); + db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_id ON " + getTableName() + " (id);"); + db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_upstreamVercode ON " + getTableName() + " (upstreamVercode);"); + db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_compatible ON " + getTableName() + " (compatible);"); } private void commitTable() { @@ -125,21 +150,22 @@ public class TempAppProvider extends AppProvider { try { db.beginTransaction(); - Log.i(TAG, "Renaming " + TABLE_TEMP_APP + " to " + DBHelper.TABLE_APP); - db.execSQL("DROP TABLE " + DBHelper.TABLE_APP); - db.execSQL("ALTER TABLE " + TABLE_TEMP_APP + " RENAME TO " + DBHelper.TABLE_APP); + final String tempApp = DB + "." + TempAppProvider.TABLE_TEMP_APP; + final String tempApk = DB + "." + TempApkProvider.TABLE_TEMP_APK; - Log.i(TAG, "Renaming " + TempApkProvider.TABLE_TEMP_APK + " to " + DBHelper.TABLE_APK); - db.execSQL("DROP TABLE " + DBHelper.TABLE_APK); - db.execSQL("ALTER TABLE " + TempApkProvider.TABLE_TEMP_APK + " RENAME TO " + DBHelper.TABLE_APK); + db.execSQL("DELETE FROM " + DBHelper.TABLE_APP + " WHERE 1"); + db.execSQL("INSERT INTO " + DBHelper.TABLE_APP + " SELECT * FROM " + tempApp); + + db.execSQL("DELETE FROM " + DBHelper.TABLE_APK + " WHERE 1"); + db.execSQL("INSERT INTO " + DBHelper.TABLE_APK + " SELECT * FROM " + tempApk); - Utils.debugLog(TAG, "Successfully renamed both tables, will commit transaction"); db.setTransactionSuccessful(); getContext().getContentResolver().notifyChange(AppProvider.getContentUri(), null); getContext().getContentResolver().notifyChange(ApkProvider.getContentUri(), null); } finally { db.endTransaction(); + db.execSQL("DETACH DATABASE " + DB); // Can't be done in a transaction. } } } From f9b22442edd6b5d7b2d3a604fa066bd3a41748c1 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 28 Apr 2016 14:54:38 +1000 Subject: [PATCH 3/4] Simplify detach code by catching exception istead of querying for attached dbs. --- .../fdroid/fdroid/data/TempAppProvider.java | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java index 86ac6bc59..d2ee5aa6e 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java @@ -3,21 +3,15 @@ package org.fdroid.fdroid.data; import android.content.ContentValues; import android.content.Context; import android.content.UriMatcher; -import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; +import android.database.sqlite.SQLiteException; import android.net.Uri; -import android.text.TextUtils; -import android.util.Log; - -import org.fdroid.fdroid.Utils; /** * This class does all of its operations in a temporary sqlite table. */ public class TempAppProvider extends AppProvider { - private static final String TAG = "TempAppProvider"; - /** * The name of the in memory database used for updating. */ @@ -120,18 +114,12 @@ public class TempAppProvider extends AppProvider { } private void ensureTempTableDetached(SQLiteDatabase db) { - Cursor cursor = db.rawQuery("PRAGMA database_list", null); try { - cursor.moveToFirst(); - while (!cursor.isAfterLast()) { - String name = cursor.getString(cursor.getColumnIndex("name")); - if (TextUtils.equals(name, DB)) { - db.execSQL("DETACH DATABASE " + DB); - } - cursor.moveToNext(); - } - } finally { - cursor.close(); + db.execSQL("DETACH DATABASE " + DB); + } catch (SQLiteException e) { + // We expect that most of the time the database will not exist unless an error occurred + // midway through the last update, The resulting exception is: + // android.database.sqlite.SQLiteException: no such database: temp_update_db (code 1) } } From ffa6fb92b9e1265e5a12408c1192ba5837fd5f6b Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 28 Apr 2016 15:06:31 +1000 Subject: [PATCH 4/4] Updated CHANGELOG with performance impovements. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19d59e431..f60125789 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ### Upcoming release +* Significant performance improvements when updating repositories + * Show what repository each apk comes from * Better support for Android 6.0