From 4d9dc4286835f42200cd294d68cca674107307f1 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Thu, 15 Dec 2022 12:53:21 -0500 Subject: [PATCH] Improve the performance of the migration by ~4x. --- .../securesms/database/MessageTable.java | 9 +++ .../securesms/database/SearchTable.kt | 35 ++++++++++++ .../V168_SingleMessageTableMigration.kt | 56 ++++++++++++++++++- .../securesms/jobs/JobManagerFactories.java | 2 + .../migrations/ApplicationMigrations.java | 12 +++- .../RebuildMessageSearchIndexMigrationJob.kt | 37 ++++++++++++ .../main/java/org/signal/core/util/SqlUtil.kt | 1 + 7 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/migrations/RebuildMessageSearchIndexMigrationJob.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.java b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.java index 2e957be8bf..b01d9f3e0d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.java @@ -3721,6 +3721,15 @@ public class MessageTable extends DatabaseTable implements MmsSmsColumns, Recipi getWritableDatabase().update(TABLE_NAME, values, THREAD_ID + " = ?", SqlUtil.buildArgs(fromId)); } + /** + * Returns the next ID that would be generated if an insert was done on this table. + * You should *not* use this for actually generating an ID to use. That will happen automatically! + * This was added for a very narrow usecase, and you probably don't need to use it. + */ + public long getNextId() { + return SqlUtil.getNextAutoIncrementId(getWritableDatabase(), TABLE_NAME); + } + void updateReactionsUnread(SQLiteDatabase db, long messageId, boolean hasReactions, boolean isRemoval) { try { boolean isOutgoing = getMessageRecord(messageId).isOutgoing(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SearchTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/SearchTable.kt index 810a2978c0..87f1e3b28e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/SearchTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/SearchTable.kt @@ -6,6 +6,7 @@ import android.database.Cursor import android.text.TextUtils import org.intellij.lang.annotations.Language import org.signal.core.util.SqlUtil +import org.signal.core.util.logging.Log /** * Contains all databases necessary for full-text search (FTS). @@ -13,6 +14,8 @@ import org.signal.core.util.SqlUtil @SuppressLint("RecipientIdDatabaseReferenceUsage", "ThreadIdDatabaseReferenceUsage") // Handles updates via triggers class SearchTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTable(context, databaseHelper) { companion object { + private val TAG = Log.tag(SearchTable::class.java) + const val MMS_FTS_TABLE_NAME = "mms_fts" const val ID = "rowid" const val BODY = MmsSmsColumns.BODY @@ -111,6 +114,38 @@ class SearchTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa } } + /** + * Re-adds every message to the index. It's fine to insert the same message twice; the table will naturally de-dupe. + * + * In order to prevent the database from locking up with super large inserts, this will perform the re-index in batches of the size you specify. + * It is not guaranteed that every batch will be the same size, but rather that the batches will be _no larger_ than the specified size. + * + * Warning: This is a potentially extremely-costly operation! It can take 10+ seconds on large installs and/or slow devices. + * Be smart about where you call this. + */ + fun rebuildIndex(batchSize: Long = 10_000L) { + val maxId: Long = SignalDatabase.messages.nextId + + Log.i(TAG, "Re-indexing. Operating on ID's 1-$maxId in steps of $batchSize.") + + for (i in 1..maxId step batchSize) { + Log.i(TAG, "Reindexing ID's [$i, ${i + batchSize})") + writableDatabase.execSQL( + """ + INSERT INTO $MMS_FTS_TABLE_NAME ($ID, $BODY) + SELECT + ${MessageTable.ID}, + ${MessageTable.BODY} + FROM + ${MessageTable.TABLE_NAME} + WHERE + ${MessageTable.ID} >= $i AND + ${MessageTable.ID} < ${i + batchSize} + """.trimIndent() + ) + } + } + private fun createFullTextSearchQuery(query: String): String { return query .split(" ") diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V168_SingleMessageTableMigration.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V168_SingleMessageTableMigration.kt index 053668769e..e1edc2a51f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V168_SingleMessageTableMigration.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V168_SingleMessageTableMigration.kt @@ -3,14 +3,42 @@ package org.thoughtcrime.securesms.database.helpers.migration import android.app.Application import net.zetetic.database.sqlcipher.SQLiteDatabase import org.signal.core.util.SqlUtil +import org.signal.core.util.Stopwatch +import org.signal.core.util.logging.Log object V168_SingleMessageTableMigration : SignalDatabaseMigration { + private val TAG = Log.tag(V168_SingleMessageTableMigration::class.java) + override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { + val stopwatch = Stopwatch("migration") + val nextMmsId = SqlUtil.getNextAutoIncrementId(db, "mms") + stopwatch.split("next-id") db.execSQL("DROP TRIGGER msl_sms_delete") db.execSQL("DROP TRIGGER reactions_sms_delete") + db.execSQL("DROP TRIGGER sms_ai") + db.execSQL("DROP TRIGGER sms_au") + db.execSQL("DROP TRIGGER sms_ad") db.execSQL("DROP TABLE sms_fts") // Will drop all other related fts tables + stopwatch.split("drop-triggers") + + // It's actually much faster to drop the indexes, copy the data, then recreate the indexes in bulk than it is to keep them and index-as-you-insert. + // Like, at least twice as fast. + db.execSQL("DROP INDEX mms_read_and_notified_and_thread_id_index") + db.execSQL("DROP INDEX mms_type_index") + db.execSQL("DROP INDEX mms_date_sent_index") + db.execSQL("DROP INDEX mms_date_server_index") + db.execSQL("DROP INDEX mms_thread_date_index") + db.execSQL("DROP INDEX mms_reactions_unread_index") + db.execSQL("DROP INDEX mms_story_type_index") + db.execSQL("DROP INDEX mms_parent_story_id_index") + db.execSQL("DROP INDEX mms_thread_story_parent_story_index") + db.execSQL("DROP INDEX mms_quote_id_quote_author_index") + db.execSQL("DROP INDEX mms_exported_index") + db.execSQL("DROP INDEX mms_id_type_payment_transactions_index") + db.execSQL("DROP TRIGGER mms_ai") // Note: For perf reasons, we won't actually rebuild the index here -- we'll rebuild it asynchronously in a job + stopwatch.split("drop-mms-indexes") db.execSQL( """ @@ -66,8 +94,31 @@ object V168_SingleMessageTableMigration : SignalDatabaseMigration { FROM sms """ ) + stopwatch.split("copy-sms") db.execSQL("DROP TABLE sms") + stopwatch.split("drop-sms") + + db.execSQL("CREATE INDEX mms_read_and_notified_and_thread_id_index ON mms(read, notified, thread_id)") + db.execSQL("CREATE INDEX mms_type_index ON mms (type)") + db.execSQL("CREATE INDEX mms_date_sent_index ON mms (date_sent, recipient_id, thread_id)") + db.execSQL("CREATE INDEX mms_date_server_index ON mms (date_server)") + db.execSQL("CREATE INDEX mms_thread_date_index ON mms (thread_id, date_received)") + db.execSQL("CREATE INDEX mms_reactions_unread_index ON mms (reactions_unread)") + db.execSQL("CREATE INDEX mms_story_type_index ON mms (story_type)") + db.execSQL("CREATE INDEX mms_parent_story_id_index ON mms (parent_story_id)") + db.execSQL("CREATE INDEX mms_thread_story_parent_story_index ON mms (thread_id, date_received, story_type, parent_story_id)") + db.execSQL("CREATE INDEX mms_quote_id_quote_author_index ON mms (quote_id, quote_author)") + db.execSQL("CREATE INDEX mms_exported_index ON mms (exported)") + db.execSQL("CREATE INDEX mms_id_type_payment_transactions_index ON mms (_id, type) WHERE type & ${0x300000000L} != 0") + db.execSQL( + """ + CREATE TRIGGER mms_ai AFTER INSERT ON mms BEGIN + INSERT INTO mms_fts (rowid, body, thread_id) VALUES (new._id, new.body, new.thread_id); + END; + """ + ) + stopwatch.split("rebuild-indexes") db.execSQL( """ @@ -76,6 +127,7 @@ object V168_SingleMessageTableMigration : SignalDatabaseMigration { WHERE is_mms = 0 """ ) + stopwatch.split("update-reactions") db.execSQL( """ @@ -84,8 +136,10 @@ object V168_SingleMessageTableMigration : SignalDatabaseMigration { WHERE is_mms = 0 """ ) + stopwatch.split("update-msl") + + stopwatch.stop(TAG) - // TODO search index? // TODO jobs? } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java index 753067b1b6..ada6b7e010 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java @@ -53,6 +53,7 @@ import org.thoughtcrime.securesms.migrations.PniAccountInitializationMigrationJo import org.thoughtcrime.securesms.migrations.PniMigrationJob; import org.thoughtcrime.securesms.migrations.ProfileMigrationJob; import org.thoughtcrime.securesms.migrations.ProfileSharingUpdateMigrationJob; +import org.thoughtcrime.securesms.migrations.RebuildMessageSearchIndexMigrationJob; import org.thoughtcrime.securesms.migrations.RecipientSearchMigrationJob; import org.thoughtcrime.securesms.migrations.RegistrationPinV2MigrationJob; import org.thoughtcrime.securesms.migrations.StickerAdditionMigrationJob; @@ -219,6 +220,7 @@ public final class JobManagerFactories { put(PniMigrationJob.KEY, new PniMigrationJob.Factory()); put(ProfileMigrationJob.KEY, new ProfileMigrationJob.Factory()); put(ProfileSharingUpdateMigrationJob.KEY, new ProfileSharingUpdateMigrationJob.Factory()); + put(RebuildMessageSearchIndexMigrationJob.KEY, new RebuildMessageSearchIndexMigrationJob.Factory()); put(RecipientSearchMigrationJob.KEY, new RecipientSearchMigrationJob.Factory()); put(RegistrationPinV2MigrationJob.KEY, new RegistrationPinV2MigrationJob.Factory()); put(StickerLaunchMigrationJob.KEY, new StickerLaunchMigrationJob.Factory()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java index db03de0ea8..7e873fa94c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java @@ -112,9 +112,11 @@ public class ApplicationMigrations { static final int STORY_VIEWED_STATE = 68; static final int STORY_READ_STATE = 69; static final int THREAD_MESSAGE_SCHEMA_CHANGE = 70; + static final int SMS_MMS_MERGE = 71; + static final int REBUILD_MESSAGE_FTS_INDEX = 72; } - public static final int CURRENT_VERSION = 70; + public static final int CURRENT_VERSION = 72; /** * This *must* be called after the {@link JobManager} has been instantiated, but *before* the call @@ -496,6 +498,14 @@ public class ApplicationMigrations { jobs.put(Version.THREAD_MESSAGE_SCHEMA_CHANGE, new DatabaseMigrationJob()); } + if (lastSeenVersion < Version.SMS_MMS_MERGE) { + jobs.put(Version.SMS_MMS_MERGE, new DatabaseMigrationJob()); + } + + if (lastSeenVersion < Version.REBUILD_MESSAGE_FTS_INDEX) { + jobs.put(Version.REBUILD_MESSAGE_FTS_INDEX, new RebuildMessageSearchIndexMigrationJob()); + } + return jobs; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/RebuildMessageSearchIndexMigrationJob.kt b/app/src/main/java/org/thoughtcrime/securesms/migrations/RebuildMessageSearchIndexMigrationJob.kt new file mode 100644 index 0000000000..3df514d6fa --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/RebuildMessageSearchIndexMigrationJob.kt @@ -0,0 +1,37 @@ +package org.thoughtcrime.securesms.migrations + +import org.signal.core.util.logging.Log +import org.thoughtcrime.securesms.database.SignalDatabase +import org.thoughtcrime.securesms.jobmanager.Data +import org.thoughtcrime.securesms.jobmanager.Job + +/** + * Rebuilds the full-text search index for the messages table. + */ +internal class RebuildMessageSearchIndexMigrationJob( + parameters: Parameters = Parameters.Builder().build() +) : MigrationJob(parameters) { + + companion object { + val TAG = Log.tag(RebuildMessageSearchIndexMigrationJob::class.java) + const val KEY = "RebuildMessageSearchIndexMigrationJob" + } + + override fun getFactoryKey(): String = KEY + + override fun isUiBlocking(): Boolean = false + + override fun performMigration() { + val startTime = System.currentTimeMillis() + SignalDatabase.messageSearch.rebuildIndex() + Log.d(TAG, "It took ${System.currentTimeMillis() - startTime} ms to rebuild the search index.") + } + + override fun shouldRetry(e: Exception): Boolean = false + + class Factory : Job.Factory { + override fun create(parameters: Parameters, data: Data): RebuildMessageSearchIndexMigrationJob { + return RebuildMessageSearchIndexMigrationJob(parameters) + } + } +} diff --git a/core-util/src/main/java/org/signal/core/util/SqlUtil.kt b/core-util/src/main/java/org/signal/core/util/SqlUtil.kt index 2a2384a775..a4daff534b 100644 --- a/core-util/src/main/java/org/signal/core/util/SqlUtil.kt +++ b/core-util/src/main/java/org/signal/core/util/SqlUtil.kt @@ -44,6 +44,7 @@ object SqlUtil { return tables } + @JvmStatic fun getNextAutoIncrementId(db: SupportSQLiteDatabase, table: String): Long { db.query("SELECT * FROM sqlite_sequence WHERE name = ?", arrayOf(table)).use { cursor -> if (cursor.moveToFirst()) {