From 530e427b79509a0ada03b03f52235e8432ce4e78 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Tue, 13 Jun 2023 08:58:08 -0600 Subject: [PATCH] music: improve library updates Update library information in a synchronized block and then only dispatch the update on the main thread. --- .../oxycblt/auxio/music/MusicRepository.kt | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt index 6fa6801fb..513786f87 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt @@ -32,6 +32,7 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.async import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import kotlinx.coroutines.yield import org.oxycblt.auxio.music.cache.CacheRepository import org.oxycblt.auxio.music.device.DeviceLibrary @@ -300,35 +301,35 @@ constructor( val userLibrary = synchronized(this) { userLibrary ?: return } logD("Creating playlist $name with ${songs.size} songs") userLibrary.createPlaylist(name, songs) - emitLibraryChange(device = false, user = true) + dispatchLibraryChange(device = false, user = true) } override suspend fun renamePlaylist(playlist: Playlist, name: String) { val userLibrary = synchronized(this) { userLibrary ?: return } logD("Renaming $playlist to $name") userLibrary.renamePlaylist(playlist, name) - emitLibraryChange(device = false, user = true) + dispatchLibraryChange(device = false, user = true) } override suspend fun deletePlaylist(playlist: Playlist) { val userLibrary = synchronized(this) { userLibrary ?: return } logD("Deleting $playlist") userLibrary.deletePlaylist(playlist) - emitLibraryChange(device = false, user = true) + dispatchLibraryChange(device = false, user = true) } override suspend fun addToPlaylist(songs: List, playlist: Playlist) { val userLibrary = synchronized(this) { userLibrary ?: return } logD("Adding ${songs.size} songs to $playlist") userLibrary.addToPlaylist(playlist, songs) - emitLibraryChange(device = false, user = true) + dispatchLibraryChange(device = false, user = true) } override suspend fun rewritePlaylist(playlist: Playlist, songs: List) { val userLibrary = synchronized(this) { userLibrary ?: return } logD("Rewriting $playlist with ${songs.size} songs") userLibrary.rewritePlaylist(playlist, songs) - emitLibraryChange(device = false, user = true) + dispatchLibraryChange(device = false, user = true) } @Synchronized @@ -353,7 +354,7 @@ constructor( // Music loading process failed due to something we have not handled. logE("Music indexing failed") logE(e.stackTraceToString()) - emitComplete(e) + emitIndexingCompletion(e) } } @@ -367,7 +368,7 @@ constructor( // Start initializing the extractors. Use an indeterminate state, as there is no ETA on // how long a media database query will take. - emitLoading(IndexingProgress.Indeterminate) + emitIndexingProgress(IndexingProgress.Indeterminate) // Do the initial query of the cache and media databases in parallel. logD("Starting MediaStore query") @@ -413,7 +414,7 @@ constructor( val rawSongs = LinkedList() for (rawSong in processedSongs) { rawSongs.add(rawSong) - emitLoading(IndexingProgress.Songs(rawSongs.size, query.projectedTotal)) + emitIndexingProgress(IndexingProgress.Songs(rawSongs.size, query.projectedTotal)) } logD("Awaiting discovery completion") // These should be no-ops, but we need the error state to see if we should keep going. @@ -428,7 +429,7 @@ constructor( // Successfully loaded the library, now save the cache and read playlist information // in parallel. logD("Discovered ${rawSongs.size} songs, starting finalization") - emitLoading(IndexingProgress.Indeterminate) + emitIndexingProgress(IndexingProgress.Indeterminate) logD("Starting UserLibrary query") val userLibraryQueryJob = worker.scope.tryAsync { userLibraryFactory.query() } if (cache == null || cache.invalidated) { @@ -443,22 +444,27 @@ constructor( val userLibrary = userLibraryFactory.create(rawPlaylists, deviceLibrary) logD("Successfully indexed music library [device=$deviceLibrary user=$userLibrary]") - emitComplete(null) + emitIndexingCompletion(null) // Comparing the library instances is obscenely expensive, do it within the library - val deviceLibraryChanged = this.deviceLibrary != deviceLibrary - val userLibraryChanged = this.userLibrary != userLibrary - if (!deviceLibraryChanged && !userLibraryChanged) { - logD("Library has not changed, skipping update") - return - } + val deviceLibraryChanged: Boolean + val userLibraryChanged: Boolean synchronized(this) { + deviceLibraryChanged = this.deviceLibrary != deviceLibrary + userLibraryChanged = this.userLibrary != userLibrary + if (!deviceLibraryChanged && !userLibraryChanged) { + logD("Library has not changed, skipping update") + return + } + this.deviceLibrary = deviceLibrary this.userLibrary = userLibrary } - emitLibraryChange(deviceLibraryChanged, userLibraryChanged) + withContext(Dispatchers.Main) { + dispatchLibraryChange(deviceLibraryChanged, userLibraryChanged) + } } /** @@ -477,7 +483,7 @@ constructor( } } - private suspend fun emitLoading(progress: IndexingProgress) { + private suspend fun emitIndexingProgress(progress: IndexingProgress) { yield() synchronized(this) { currentIndexingState = IndexingState.Indexing(progress) @@ -487,7 +493,7 @@ constructor( } } - private suspend fun emitComplete(error: Exception?) { + private suspend fun emitIndexingCompletion(error: Exception?) { yield() synchronized(this) { previousCompletedState = IndexingState.Completed(error) @@ -500,7 +506,7 @@ constructor( } @Synchronized - private fun emitLibraryChange(device: Boolean, user: Boolean) { + private fun dispatchLibraryChange(device: Boolean, user: Boolean) { val changes = MusicRepository.Changes(device, user) logD("Dispatching library change [changes=$changes]") for (listener in updateListeners) {