Merge branch 'fix-506--rename-temp-table' into 'master'
Speed up "Saving Application Details" part of repo update Fixes 506. This does the thing mentioned in 506 (renaming temp table, rather than copying data). In the process, I also identified that the temp tables were missing key indexes which slowed the process down. This fix took the update time from ~100 seconds to ~60 seconds on my Nexus 4. Below are a couple of log cats from before and after the change (this logging is not part of this MR, it was just for diagnosing the problem). *Before change:* ``` AppProvider D Calculating whether apps are compatible, based on whether any of their apks are compatible D Update compatible flags took 20244ms D Calculating suggested versions for all apps which specify an upstream version code. D Update suggested from upstream took 17808ms D Calculating suggested versions for all apps which don't specify an upstream version code. D Update suggested from latest took 129ms AppProvider D Update icon URLs took 7879ms UpdateService I Updating repo(s) complete, took 104 seconds to complete. ``` *After change:* ``` AppProvider D Calculating whether apps are compatible, based on whether any of their apks are compatible D Update compatible flags took 1047ms D Calculating suggested versions for all apps which specify an upstream version code. D Update suggested from upstream took 601ms D Calculating suggested versions for all apps which don't specify an upstream version code. D Update suggested from latest took 136ms D Update icon URLs took 887ms UpdateService I Updating repo(s) complete, took 63 seconds to complete. ``` See merge request !179
This commit is contained in:
commit
9950ea3ed2
@ -391,7 +391,6 @@ public class UpdateService extends IntentService implements ProgressListener {
|
|||||||
if (!changes) {
|
if (!changes) {
|
||||||
Utils.debugLog(TAG, "Not checking app details or compatibility, because all repos were up to date.");
|
Utils.debugLog(TAG, "Not checking app details or compatibility, because all repos were up to date.");
|
||||||
} else {
|
} else {
|
||||||
sendStatus(this, STATUS_INFO, getString(R.string.status_checking_compatibility));
|
|
||||||
notifyContentProviders();
|
notifyContentProviders();
|
||||||
|
|
||||||
if (prefs.getBoolean(Preferences.PREF_UPD_NOTIFY, true)) {
|
if (prefs.getBoolean(Preferences.PREF_UPD_NOTIFY, true)) {
|
||||||
|
@ -62,9 +62,13 @@ public class RepoPersister {
|
|||||||
@NonNull
|
@NonNull
|
||||||
private final Map<String, List<Apk>> apksToSave = new HashMap<>();
|
private final Map<String, List<Apk>> apksToSave = new HashMap<>();
|
||||||
|
|
||||||
|
@NonNull
|
||||||
|
private final CompatibilityChecker checker;
|
||||||
|
|
||||||
public RepoPersister(@NonNull Context context, @NonNull Repo repo) {
|
public RepoPersister(@NonNull Context context, @NonNull Repo repo) {
|
||||||
this.repo = repo;
|
this.repo = repo;
|
||||||
this.context = context;
|
this.context = context;
|
||||||
|
checker = new CompatibilityChecker(context);
|
||||||
}
|
}
|
||||||
|
|
||||||
public void saveToDb(App app, List<Apk> packages) throws RepoUpdater.UpdateException {
|
public void saveToDb(App app, List<Apk> packages) throws RepoUpdater.UpdateException {
|
||||||
@ -78,8 +82,7 @@ public class RepoPersister {
|
|||||||
|
|
||||||
public void commit(ContentValues repoDetailsToSave) throws RepoUpdater.UpdateException {
|
public void commit(ContentValues repoDetailsToSave) throws RepoUpdater.UpdateException {
|
||||||
flushBufferToDb();
|
flushBufferToDb();
|
||||||
TempAppProvider.Helper.commit(context);
|
TempAppProvider.Helper.commitAppsAndApks(context);
|
||||||
TempApkProvider.Helper.commit(context);
|
|
||||||
RepoProvider.Helper.update(context, repo, repoDetailsToSave);
|
RepoProvider.Helper.update(context, repo, repoDetailsToSave);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -282,7 +285,6 @@ public class RepoPersister {
|
|||||||
* in order to see if, and why an apk is not compatible.
|
* in order to see if, and why an apk is not compatible.
|
||||||
*/
|
*/
|
||||||
private void calcApkCompatibilityFlags(List<Apk> apks) {
|
private void calcApkCompatibilityFlags(List<Apk> apks) {
|
||||||
final CompatibilityChecker checker = new CompatibilityChecker(context);
|
|
||||||
for (final Apk apk : apks) {
|
for (final Apk apk : apks) {
|
||||||
final List<String> reasons = checker.getIncompatibleReasons(apk);
|
final List<String> reasons = checker.getIncompatibleReasons(apk);
|
||||||
if (reasons.size() > 0) {
|
if (reasons.size() > 0) {
|
||||||
|
@ -17,26 +17,23 @@ public class TempApkProvider extends ApkProvider {
|
|||||||
|
|
||||||
private static final String PROVIDER_NAME = "TempApkProvider";
|
private static final String PROVIDER_NAME = "TempApkProvider";
|
||||||
|
|
||||||
static final String TABLE_TEMP_APK = "temp_fdroid_apk";
|
static final String TABLE_TEMP_APK = "temp_" + DBHelper.TABLE_APK;
|
||||||
|
|
||||||
private static final String PATH_INIT = "init";
|
private static final String PATH_INIT = "init";
|
||||||
private static final String PATH_COMMIT = "commit";
|
|
||||||
|
|
||||||
private static final int CODE_INIT = 10000;
|
private static final int CODE_INIT = 10000;
|
||||||
private static final int CODE_COMMIT = CODE_INIT + 1;
|
|
||||||
|
|
||||||
private static final UriMatcher matcher = new UriMatcher(-1);
|
private static final UriMatcher matcher = new UriMatcher(-1);
|
||||||
|
|
||||||
static {
|
static {
|
||||||
matcher.addURI(getAuthority(), PATH_INIT, CODE_INIT);
|
matcher.addURI(getAuthority(), PATH_INIT, CODE_INIT);
|
||||||
matcher.addURI(getAuthority(), PATH_COMMIT, CODE_COMMIT);
|
|
||||||
matcher.addURI(getAuthority(), PATH_APK + "/#/*", CODE_SINGLE);
|
matcher.addURI(getAuthority(), PATH_APK + "/#/*", CODE_SINGLE);
|
||||||
matcher.addURI(getAuthority(), PATH_REPO_APK + "/#/*", CODE_REPO_APK);
|
matcher.addURI(getAuthority(), PATH_REPO_APK + "/#/*", CODE_REPO_APK);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected String getTableName() {
|
protected String getTableName() {
|
||||||
return "temp_" + super.getTableName();
|
return TABLE_TEMP_APK;
|
||||||
}
|
}
|
||||||
|
|
||||||
public static String getAuthority() {
|
public static String getAuthority() {
|
||||||
@ -76,15 +73,6 @@ public class TempApkProvider extends ApkProvider {
|
|||||||
context.getContentResolver().insert(uri, new ContentValues());
|
context.getContentResolver().insert(uri, new ContentValues());
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Saves data from the temp table to the apk table, by removing _EVERYTHING_ from the real
|
|
||||||
* apk table and inserting all of the records from here. The temporary table is then removed.
|
|
||||||
*/
|
|
||||||
public static void commit(Context context) {
|
|
||||||
Uri uri = Uri.withAppendedPath(getContentUri(), PATH_COMMIT);
|
|
||||||
context.getContentResolver().insert(uri, new ContentValues());
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@ -93,9 +81,6 @@ public class TempApkProvider extends ApkProvider {
|
|||||||
case CODE_INIT:
|
case CODE_INIT:
|
||||||
initTable();
|
initTable();
|
||||||
return null;
|
return null;
|
||||||
case CODE_COMMIT:
|
|
||||||
commitTable();
|
|
||||||
return null;
|
|
||||||
default:
|
default:
|
||||||
return super.insert(uri, values);
|
return super.insert(uri, values);
|
||||||
}
|
}
|
||||||
@ -138,12 +123,9 @@ public class TempApkProvider extends ApkProvider {
|
|||||||
private void initTable() {
|
private void initTable() {
|
||||||
write().execSQL("DROP TABLE IF EXISTS " + getTableName());
|
write().execSQL("DROP TABLE IF EXISTS " + getTableName());
|
||||||
write().execSQL("CREATE TABLE " + getTableName() + " AS SELECT * FROM " + DBHelper.TABLE_APK);
|
write().execSQL("CREATE TABLE " + getTableName() + " AS SELECT * FROM " + DBHelper.TABLE_APK);
|
||||||
|
write().execSQL("CREATE INDEX IF NOT EXISTS apk_vercode on " + getTableName() + " (vercode);");
|
||||||
|
write().execSQL("CREATE INDEX IF NOT EXISTS apk_id on " + getTableName() + " (id);");
|
||||||
|
write().execSQL("CREATE INDEX IF NOT EXISTS apk_compatible ON " + getTableName() + " (compatible);");
|
||||||
}
|
}
|
||||||
|
|
||||||
private void commitTable() {
|
|
||||||
Log.i(TAG, "Deleting all apks from " + DBHelper.TABLE_APK + " so they can be copied from " + getTableName());
|
|
||||||
write().execSQL("DELETE FROM " + DBHelper.TABLE_APK);
|
|
||||||
write().execSQL("INSERT INTO " + DBHelper.TABLE_APK + " SELECT * FROM " + getTableName());
|
|
||||||
getContext().getContentResolver().notifyChange(ApkProvider.getContentUri(), null);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
@ -3,9 +3,12 @@ package org.fdroid.fdroid.data;
|
|||||||
import android.content.ContentValues;
|
import android.content.ContentValues;
|
||||||
import android.content.Context;
|
import android.content.Context;
|
||||||
import android.content.UriMatcher;
|
import android.content.UriMatcher;
|
||||||
|
import android.database.sqlite.SQLiteDatabase;
|
||||||
import android.net.Uri;
|
import android.net.Uri;
|
||||||
import android.util.Log;
|
import android.util.Log;
|
||||||
|
|
||||||
|
import org.fdroid.fdroid.Utils;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* This class does all of its operations in a temporary sqlite table.
|
* This class does all of its operations in a temporary sqlite table.
|
||||||
*/
|
*/
|
||||||
@ -15,7 +18,7 @@ public class TempAppProvider extends AppProvider {
|
|||||||
|
|
||||||
private static final String PROVIDER_NAME = "TempAppProvider";
|
private static final String PROVIDER_NAME = "TempAppProvider";
|
||||||
|
|
||||||
private static final String TABLE_TEMP_APP = "temp_fdroid_app";
|
private static final String TABLE_TEMP_APP = "temp_" + DBHelper.TABLE_APP;
|
||||||
|
|
||||||
private static final String PATH_INIT = "init";
|
private static final String PATH_INIT = "init";
|
||||||
private static final String PATH_COMMIT = "commit";
|
private static final String PATH_COMMIT = "commit";
|
||||||
@ -63,11 +66,10 @@ public class TempAppProvider extends AppProvider {
|
|||||||
* Saves data from the temp table to the apk table, by removing _EVERYTHING_ from the real
|
* Saves data from the temp table to the apk table, by removing _EVERYTHING_ from the real
|
||||||
* apk table and inserting all of the records from here. The temporary table is then removed.
|
* apk table and inserting all of the records from here. The temporary table is then removed.
|
||||||
*/
|
*/
|
||||||
public static void commit(Context context) {
|
public static void commitAppsAndApks(Context context) {
|
||||||
Uri uri = Uri.withAppendedPath(getContentUri(), PATH_COMMIT);
|
Uri uri = Uri.withAppendedPath(getContentUri(), PATH_COMMIT);
|
||||||
context.getContentResolver().insert(uri, new ContentValues());
|
context.getContentResolver().insert(uri, new ContentValues());
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@ -112,12 +114,31 @@ public class TempAppProvider extends AppProvider {
|
|||||||
private void initTable() {
|
private void initTable() {
|
||||||
write().execSQL("DROP TABLE IF EXISTS " + getTableName());
|
write().execSQL("DROP TABLE IF EXISTS " + getTableName());
|
||||||
write().execSQL("CREATE TABLE " + getTableName() + " AS SELECT * FROM " + DBHelper.TABLE_APP);
|
write().execSQL("CREATE TABLE " + getTableName() + " AS SELECT * FROM " + DBHelper.TABLE_APP);
|
||||||
|
write().execSQL("CREATE INDEX IF NOT EXISTS app_id ON " + getTableName() + " (id);");
|
||||||
|
write().execSQL("CREATE INDEX IF NOT EXISTS app_upstreamVercode ON " + getTableName() + " (upstreamVercode);");
|
||||||
|
write().execSQL("CREATE INDEX IF NOT EXISTS app_compatible ON " + getTableName() + " (compatible);");
|
||||||
}
|
}
|
||||||
|
|
||||||
private void commitTable() {
|
private void commitTable() {
|
||||||
Log.i(TAG, "Deleting all apks from " + DBHelper.TABLE_APP + " so they can be copied from " + getTableName());
|
final SQLiteDatabase db = write();
|
||||||
write().execSQL("DELETE FROM " + DBHelper.TABLE_APP);
|
try {
|
||||||
write().execSQL("INSERT INTO " + DBHelper.TABLE_APP + " SELECT * FROM " + getTableName());
|
db.beginTransaction();
|
||||||
getContext().getContentResolver().notifyChange(AppProvider.getContentUri(), null);
|
|
||||||
|
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);
|
||||||
|
|
||||||
|
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);
|
||||||
|
|
||||||
|
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();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user