Optimize multi-artist processing: cache split results and remove redundant operations by Copilot · Pull Request #549 · theovilardo/PixelPlayer

Copilot · 2025-12-16T21:18:27Z

Addressed performance and code quality issues in the multi-artist implementation where artist name splitting was performed redundantly across multiple passes through the song list.

Performance optimization

Cache artist name splits in songToArtists map, reusing results across all operations
Combine preprocessing and ID assignment into single pass
Remove redundant split in album creation (corrected songs already contain primary artist)

Code quality

Add validation in setArtistDelimiters() to enforce at least one delimiter
Remove unused preProcessAndDeduplicate() legacy function

Before:

// First pass: split and cache
songs.forEach { song ->
    val artists = song.artistName.splitArtistsByDelimiters(delimiters)
    cache[song.id] = artists
}

// Second pass: assign IDs (redundant split avoided)
songs.forEach { song ->
    val artists = cache[song.id] ?: listOf(song.artistName)
    // ...
}

// Album deduplication: splitting again
songs.forEach { song ->
    val primaryArtist = song.artistName.splitArtistsByDelimiters(delimiters).firstOrNull()
    // ...
}

// Song correction: splitting yet again
songs.map { song ->
    val artists = song.artistName.splitArtistsByDelimiters(delimiters)
    // ...
}

After:

// Single pass: split once, cache, and assign IDs
songs.forEach { song ->
    val artists = if (artistSeparationEnabled) {
        song.artistName.splitArtistsByDelimiters(delimiters)
    } else {
        listOf(song.artistName)
    }
    cache[song.id] = artists
    
    // Assign IDs immediately
    artists.forEach { /* ... */ }
}

// All subsequent operations use cache
val primaryArtist = cache[song.id]?.firstOrNull() ?: song.artistName

Particularly beneficial for large music libraries where string splitting overhead compounds with collection size.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: lostf1sh <[email protected]>

Copilot · 2025-12-16T21:25:57Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

dl.google.com
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:+UseParallelGC -Xmx6g -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Copilot

Pull request overview

This PR optimizes the multi-artist processing implementation by introducing a caching mechanism for artist name splits, eliminating redundant string processing operations across multiple passes through the song list. The optimization consolidates artist splitting logic and adds input validation for delimiter configuration.

Implements caching of artist name splits in a songToArtists map to avoid redundant string processing
Removes redundant artist splitting operations in album deduplication and song correction phases
Adds validation to ensure at least one delimiter is maintained when updating artist delimiters
Removes unused legacy preprocessing function

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
SyncWorker.kt	Introduces songToArtists cache map to store split results, eliminates redundant splitArtistsByDelimiters calls in album and song processing, removes unused preProcessAndDeduplicate function
UserPreferencesRepository.kt	Adds empty list validation in setArtistDelimiters to enforce at least one delimiter requirement

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-16T21:30:07Z

app/src/main/java/com/theveloper/pixelplay/data/worker/SyncWorker.kt

-            } else {
-                listOf(song.artistName)
-            }
+            val artistsForSong = songToArtists[song.id] ?: listOf(song.artistName)


Similar to the album deduplication logic, the fallback ?: listOf(song.artistName) is defensive but unnecessary since the cache is populated for all songs. This fallback could mask issues if the cache is somehow incomplete. Consider adding an assertion or logging to detect if this fallback is ever triggered.

Suggested change

val artistsForSong = songToArtists[song.id] ?: listOf(song.artistName)

val artistsForSong = checkNotNull(songToArtists[song.id]) { "songToArtists cache missing for song id: ${song.id}, name: ${song.title}" }

Copilot · 2025-12-16T21:30:07Z

app/src/main/java/com/theveloper/pixelplay/data/preferences/UserPreferencesRepository.kt

+        // Ensure at least one delimiter is always maintained
+        if (delimiters.isEmpty()) {
+            return
+        }


Silent failure when empty delimiters are provided can lead to confusing behavior. When a user tries to set empty delimiters, the function returns silently without any indication of failure or reverting to defaults. This means the rescan flag won't be set even though the operation was requested, potentially leaving the UI in an inconsistent state.

Consider either:

Throwing an IllegalArgumentException to signal that empty delimiters are not allowed

Logging a warning to help with debugging

Falling back to DEFAULT_ARTIST_DELIMITERS instead of silently ignoring the request

Copilot · 2025-12-16T21:30:08Z

app/src/main/java/com/theveloper/pixelplay/data/worker/SyncWorker.kt

+                val artistsToProcess = songToArtists[song.id] ?: listOf(song.artistName)
+                artistsToProcess.firstOrNull() ?: song.artistName


The fallback expression for cache misses is inconsistent with the cache population logic. The cache is populated for all songs in line 171, so songToArtists[song.id] should never be null when processing the same songs list. The fallback ?: listOf(song.artistName) is defensive but unnecessary and could mask issues if the cache is somehow incomplete.

Consider adding an assertion or logging to detect if this fallback is ever triggered, which would indicate a logic error in the cache population.

Suggested change

val artistsToProcess = songToArtists[song.id] ?: listOf(song.artistName)

artistsToProcess.firstOrNull() ?: song.artistName

val artistsToProcess = songToArtists[song.id]

if (artistsToProcess == null) {

Log.e("SyncWorker", "songToArtists cache miss for song id=${song.id}, name='${song.artistName}'")

throw IllegalStateException("songToArtists cache miss for song id=${song.id}, name='${song.artistName}'")

}

artistsToProcess!!.firstOrNull() ?: song.artistName

Initial plan

8a2234d

Copilot AI assigned Copilot and lostf1sh Dec 16, 2025

Copilot AI mentioned this pull request Dec 16, 2025

Multi artist support #548

Merged

Copilot started work on behalf of lostf1sh December 16, 2025 21:18 View session

Copilot AI and others added 5 commits December 16, 2025 21:20

Fix: Cache split artist results to avoid redundant string processing

efecee1

Co-authored-by: lostf1sh <[email protected]>

Add validation to ensure at least one delimiter is always maintained

93660a5

Co-authored-by: lostf1sh <[email protected]>

Remove unused preProcessAndDeduplicate legacy function

3c378c6

Co-authored-by: lostf1sh <[email protected]>

Remove redundant split in album creation logic

e5b778e

Co-authored-by: lostf1sh <[email protected]>

Optimize: Combine preprocessing and first pass into single loop

64ed048

Co-authored-by: lostf1sh <[email protected]>

lostf1sh approved these changes Dec 16, 2025

View reviewed changes

Copilot AI changed the title ~~[WIP] Add multi artist support to music database~~ Optimize multi-artist processing: cache split results and remove redundant operations Dec 16, 2025

Copilot AI requested a review from lostf1sh December 16, 2025 21:26

Copilot finished work on behalf of lostf1sh December 16, 2025 21:26

lostf1sh approved these changes Dec 16, 2025

View reviewed changes

lostf1sh marked this pull request as ready for review December 16, 2025 21:27

Copilot AI review requested due to automatic review settings December 16, 2025 21:27

Copilot started reviewing on behalf of lostf1sh December 16, 2025 21:28 View session

Copilot AI reviewed Dec 16, 2025

View reviewed changes

lostf1sh merged commit a63b00c into multi-artist-support Dec 16, 2025
6 checks passed

lostf1sh deleted the copilot/sub-pr-548 branch January 16, 2026 20:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize multi-artist processing: cache split results and remove redundant operations#549

Optimize multi-artist processing: cache split results and remove redundant operations#549
lostf1sh merged 6 commits intomulti-artist-supportfrom
copilot/sub-pr-548

Copilot AI commented Dec 16, 2025 •

edited

Loading

Uh oh!

Copilot AI commented Dec 16, 2025

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	val artistsForSong = songToArtists[song.id] ?: listOf(song.artistName)
	val artistsForSong = checkNotNull(songToArtists[song.id]) { "songToArtists cache missing for song id: ${song.id}, name: ${song.title}" }

		val artistsToProcess = songToArtists[song.id] ?: listOf(song.artistName)
		artistsToProcess.firstOrNull() ?: song.artistName

-                val artistsToProcess = songToArtists[song.id] ?: listOf(song.artistName)
-                artistsToProcess.firstOrNull() ?: song.artistName
+                val artistsToProcess = songToArtists[song.id]
+                if (artistsToProcess == null) {
+                    Log.e("SyncWorker", "songToArtists cache miss for song id=${song.id}, name='${song.artistName}'")
+                    throw IllegalStateException("songToArtists cache miss for song id=${song.id}, name='${song.artistName}'")
+                }
+                artistsToProcess!!.firstOrNull() ?: song.artistName

Conversation

Copilot AI commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI commented Dec 16, 2025

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Dec 16, 2025 •

edited

Loading