Improve the performance of setArtwork(Iterable)

Rather than doing the delete operations on a row by row level as a separate batch operation, apply it as part of the same set of operations using selection criteria. Found that this improves the cases found in #623 even more.
muzei · Jul 11, 2020 · 98735a7 · 98735a7
1 parent c0b42d9
commit 98735a7
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 42 deletions.
diff --git a/muzei-api/src/main/java/com/google/android/apps/muzei/api/provider/MuzeiArtProvider.kt b/muzei-api/src/main/java/com/google/android/apps/muzei/api/provider/MuzeiArtProvider.kt
@@ -342,29 +342,18 @@ abstract class MuzeiArtProvider : ContentProvider(), ProviderClient {
                     .withValues(art.toContentValues())
                     .build())
         }
+        val artworkCount = operations.size
+        val resultUris = ArrayList<Uri>(artworkCount)
+
+        // Delete any artwork that was not inserted/update in the above operations
         val currentTime = System.currentTimeMillis()
-        val resultUris = ArrayList<Uri>(operations.size)
+        operations.add(ContentProviderOperation.newDelete(contentUri)
+                .withSelection("${ProviderContract.Artwork.DATE_MODIFIED} < ?",
+                        arrayOf(currentTime.toString()))
+                .build())
         try {
-            resultUris.addAll(applyBatch(operations).mapNotNull { result -> result.uri })
-            val deleteOperations = ArrayList<ContentProviderOperation>()
-            query(
-                    contentUri,
-                    arrayOf(BaseColumns._ID),
-                    ProviderContract.Artwork.DATE_MODIFIED + "<?",
-                    arrayOf(currentTime.toString()), null).use { data ->
-                while (data.moveToNext()) {
-                    val artworkUri = ContentUris.withAppendedId(contentUri,
-                            data.getLong(0))
-                    if (!resultUris.contains(artworkUri)) {
-                        deleteOperations.add(ContentProviderOperation
-                                .newDelete(artworkUri)
-                                .build())
-                    }
-                }
-            }
-            if (deleteOperations.isNotEmpty()) {
-                applyBatch(deleteOperations)
-            }
+            val results = applyBatch(operations)
+            resultUris.addAll(results.take(artworkCount).mapNotNull { result -> result.uri })
         } catch (e: OperationApplicationException) {
             if (Log.isLoggable(TAG, Log.INFO)) {
                 Log.i(TAG, "setArtwork failed", e)

diff --git a/muzei-api/src/main/java/com/google/android/apps/muzei/api/provider/ProviderContract.kt b/muzei-api/src/main/java/com/google/android/apps/muzei/api/provider/ProviderContract.kt
@@ -185,30 +185,18 @@ object ProviderContract {
                             .withValues(art.toContentValues())
                             .build())
                 }
+                val artworkCount = operations.size
+                val resultUris = ArrayList<Uri>(artworkCount)
+
+                // Delete any artwork that was not inserted/update in the above operations
                 val currentTime = System.currentTimeMillis()
-                val resultUris = ArrayList<Uri>(operations.size)
+                operations.add(ContentProviderOperation.newDelete(contentUri)
+                        .withSelection("${Artwork.DATE_MODIFIED} < ?",
+                                arrayOf(currentTime.toString()))
+                        .build())
                 try {
                     val results = contentResolver.applyBatch(authority, operations)
-                    resultUris.addAll(results.mapNotNull { result -> result.uri })
-                    val deleteOperations = ArrayList<ContentProviderOperation>()
-                    context.contentResolver.query(
-                            contentUri,
-                            arrayOf(BaseColumns._ID),
-                            "${Artwork.DATE_MODIFIED} < ?",
-                            arrayOf(currentTime.toString()), null)?.use { data ->
-                        while (data.moveToNext()) {
-                            val artworkUri = ContentUris.withAppendedId(contentUri,
-                                    data.getLong(0))
-                            if (!resultUris.contains(artworkUri)) {
-                                deleteOperations.add(ContentProviderOperation
-                                        .newDelete(artworkUri)
-                                        .build())
-                            }
-                        }
-                    }
-                    if (deleteOperations.isNotEmpty()) {
-                        contentResolver.applyBatch(authority, deleteOperations)
-                    }
+                    resultUris.addAll(results.take(artworkCount).mapNotNull { result -> result.uri })
                 } catch (ignored: OperationApplicationException) {
                 } catch (ignored: RemoteException) {
                 }