From de149cf589c449551a824d85aeb8c467cb06a0b4 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 9 Jun 2017 14:46:05 +1000 Subject: [PATCH] Remove subselect and use better index. The main problem is that we were using an index on fdroid_apk.vercode, when it should have been using an index on fdroid_apk.appId. There are thousands of apks which would match based on vercode, but only two or three which match based on appId. This improves performance of the calculate-suggested-vercode query from 25,000ms to 100ms. --- .../org/fdroid/fdroid/data/AppProvider.java | 44 ++++++------------- .../fdroid/fdroid/data/TempApkProvider.java | 2 +- 2 files changed, 15 insertions(+), 31 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 47733d7f2..b803bc95f 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -961,9 +961,9 @@ public class AppProvider extends FDroidProvider { private void updateSuggestedFromUpstream() { Utils.debugLog(TAG, "Calculating suggested versions for all NON-INSTALLED apps which specify an upstream version code."); - Utils.Profiler profiler = new Utils.Profiler("UpdateSuggestedApks"); final String apk = getApkTableName(); final String app = getTableName(); + final String installed = InstalledAppTable.NAME; final boolean unstableUpdates = Preferences.get().getUnstableUpdates(); String restrictToStable = unstableUpdates ? "" : (apk + "." + ApkTable.Cols.VERSION_CODE + " <= " + app + "." + Cols.UPSTREAM_VERSION_CODE + " AND "); @@ -974,20 +974,28 @@ public class AppProvider extends FDroidProvider { // By adding the extra join, and then joining based on the packageId of this inner app table // and the app table we are updating, we take into account all apks for this app. + // The check apk.sig = COALESCE(installed.sig, apk.sig) would ideally be better written as: + // `installedSig IS NULL OR installedSig = apk.sig` + // however that would require a separate sub query for each `installedSig` which is more + // expensive. Using a COALESCE is a less expressive way to write the same thing with only + // a single subquery. + // Also note that the `installedSig IS NULL` is not because there is a `NULL` entry in the + // installed table (this is impossible), but rather because the subselect above returned + // zero rows. String updateSql = "UPDATE " + app + " SET " + Cols.SUGGESTED_VERSION_CODE + " = ( " + " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " + " FROM " + apk + " JOIN " + app + " AS appForThisApk ON (appForThisApk." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + ") " + + " LEFT JOIN " + installed + " ON (" + installed + "." + InstalledAppTable.Cols.PACKAGE_ID + " = " + app + "." + Cols.PACKAGE_ID + ") " + " WHERE " + - restrictToSameSigIfInstalled(app, apk) + " AND " + app + "." + Cols.PACKAGE_ID + " = appForThisApk." + Cols.PACKAGE_ID + " AND " + + apk + "." + ApkTable.Cols.SIGNATURE + " = COALESCE(" + installed + "." + InstalledAppTable.Cols.SIGNATURE + ", " + apk + "." + ApkTable.Cols.SIGNATURE + ") AND " + restrictToStable + " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + Cols.IS_COMPATIBLE + " = 1 ) ) " + " WHERE " + Cols.UPSTREAM_VERSION_CODE + " > 0 "; LoggingQuery.execSQL(db(), updateSql); - profiler.log("Done"); } /** @@ -1005,47 +1013,23 @@ public class AppProvider extends FDroidProvider { final String apk = getApkTableName(); final String app = getTableName(); + final String installed = InstalledAppTable.NAME; String updateSql = "UPDATE " + app + " SET " + Cols.SUGGESTED_VERSION_CODE + " = ( " + " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " + " FROM " + apk + " JOIN " + app + " AS appForThisApk ON (appForThisApk." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + ") " + + " LEFT JOIN " + installed + " ON (" + installed + "." + InstalledAppTable.Cols.PACKAGE_ID + " = " + app + "." + Cols.PACKAGE_ID + ") " + " WHERE " + - restrictToSameSigIfInstalled(app, apk) + " AND " + app + "." + Cols.PACKAGE_ID + " = appForThisApk." + Cols.PACKAGE_ID + " AND " + + apk + "." + ApkTable.Cols.SIGNATURE + " = COALESCE(" + installed + "." + InstalledAppTable.Cols.SIGNATURE + ", " + apk + "." + ApkTable.Cols.SIGNATURE + ") AND " + " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + ApkTable.Cols.IS_COMPATIBLE + " = 1 ) ) " + " WHERE COALESCE(" + Cols.UPSTREAM_VERSION_CODE + ", 0) = 0 OR " + Cols.SUGGESTED_VERSION_CODE + " IS NULL "; LoggingQuery.execSQL(db(), updateSql); } - /** - * Limits results for an apk query. If the app in question is installed, then will limit apk - * results to those matching the same signature as the installed one. Otherwise, allows all apks - * to be returned. - */ - private static String restrictToSameSigIfInstalled(String appTable, String apkTable) { - String installedSig = - "(SELECT installed." + InstalledAppTable.Cols.SIGNATURE + - " FROM " + InstalledAppTable.NAME + " AS installed " + - " JOIN " + PackageTable.NAME + " AS pkg ON " + - " (pkg." + PackageTable.Cols.ROW_ID + " = " + appTable + "." + Cols.PACKAGE_ID + " AND " + - " installed." + InstalledAppTable.Cols.PACKAGE_ID + " = pkg." + PackageTable.Cols.ROW_ID + ") " + - ")"; - - // Ideally, the check below would actually be written as: - // `installedSig IS NULL OR installedSig = apk.sig` - // however that would require a separate sub query for each `installedSig` which is more - // expensive. Using a COALESCE is a less expressive way to write the same thing with only - // a single subquery. - // Also note that the `installedSig IS NULL` is not because there is a `NULL` entry in the - // installed table (this is impossible), but rather because the subselect above returned - // zero rows. - return apkTable + "." + ApkTable.Cols.SIGNATURE + " = " + - "COALESCE(" + installedSig + ", " + apkTable + "." + ApkTable.Cols.SIGNATURE + ")"; - } - private void updateIconUrls() { final String appTable = getTableName(); final String apkTable = getApkTableName(); 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 d5a688022..412688121 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java @@ -129,7 +129,7 @@ public class TempApkProvider extends ApkProvider { final String memoryDbName = TempAppProvider.DB; db.execSQL(DBHelper.CREATE_TABLE_APK.replaceFirst(Schema.ApkTable.NAME, memoryDbName + "." + getTableName())); db.execSQL(TempAppProvider.copyData(Schema.ApkTable.Cols.ALL_COLS, Schema.ApkTable.NAME, memoryDbName + "." + getTableName())); - db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_vercode on " + getTableName() + " (" + ApkTable.Cols.VERSION_CODE + ");"); + db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_appId on " + getTableName() + " (" + ApkTable.Cols.APP_ID + ");"); db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + ApkTable.Cols.IS_COMPATIBLE + ");"); }