perf(smartplaylist): batch-load cover art + MBIDs instead of per-row subquery
The autotag overhaul added cover-art and MusicBrainz-ID columns to leanTrackQuery to support the new track-row styling, reintroducing the per-row correlated subquery anti-pattern (artist_mbid) plus a cover_art join inside the whole-library derived table. Both ran for every track before WHERE/LIMIT, so smart-playlist evaluation cost scaled with library size rather than result size — several seconds for a 500-track playlist that was previously sub-second. Move these presentation-only fields into a batched fetchArtwork pass keyed by the matched recording_ids, mirroring the existing fetchGenres batch. Cost is now proportional to results. Add TestEvaluate_ArtworkEnrichment (no prior coverage of these fields) and an artwork_ms debug metric. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -555,11 +555,7 @@ const leanTrackQuery = `SELECT
|
|||||||
af.bitrate,
|
af.bitrate,
|
||||||
af.file_size,
|
af.file_size,
|
||||||
af.play_count,
|
af.play_count,
|
||||||
COALESCE(af.last_played, '') AS last_played,
|
COALESCE(af.last_played, '') AS last_played
|
||||||
af.cover_art_path,
|
|
||||||
af.artist_mbid,
|
|
||||||
af.release_group_mbid,
|
|
||||||
af.recording_mbid
|
|
||||||
FROM (
|
FROM (
|
||||||
SELECT
|
SELECT
|
||||||
af.id,
|
af.id,
|
||||||
@@ -591,15 +587,7 @@ FROM (
|
|||||||
af.file_size,
|
af.file_size,
|
||||||
af.library_id,
|
af.library_id,
|
||||||
af.play_count,
|
af.play_count,
|
||||||
af.last_played,
|
af.last_played
|
||||||
COALESCE(ca.file_path, '') AS cover_art_path,
|
|
||||||
COALESCE((SELECT a.mbid
|
|
||||||
FROM artist_credit_artist aca
|
|
||||||
JOIN artists a ON a.id = aca.artist_id
|
|
||||||
WHERE aca.credit_id = ac.id
|
|
||||||
LIMIT 1), '') AS artist_mbid,
|
|
||||||
COALESCE(rg.mbid, '') AS release_group_mbid,
|
|
||||||
COALESCE(r.mbid, '') AS recording_mbid
|
|
||||||
FROM audio_files af
|
FROM audio_files af
|
||||||
LEFT JOIN recordings r ON af.recording_id = r.id
|
LEFT JOIN recordings r ON af.recording_id = r.id
|
||||||
LEFT JOIN artist_credit ac ON r.artist_credit_id = ac.id
|
LEFT JOIN artist_credit ac ON r.artist_credit_id = ac.id
|
||||||
@@ -610,7 +598,6 @@ FROM (
|
|||||||
GROUP BY recording_id
|
GROUP BY recording_id
|
||||||
) rgr ON r.id = rgr.recording_id
|
) rgr ON r.id = rgr.recording_id
|
||||||
LEFT JOIN release_groups rg ON rgr.release_group_id = rg.id
|
LEFT JOIN release_groups rg ON rgr.release_group_id = rg.id
|
||||||
LEFT JOIN cover_art ca ON rg.cover_art_id = ca.id
|
|
||||||
LEFT JOIN file_types ft ON af.file_type_id = ft.id
|
LEFT JOIN file_types ft ON af.file_type_id = ft.id
|
||||||
) af`
|
) af`
|
||||||
|
|
||||||
@@ -719,6 +706,38 @@ func Evaluate(
|
|||||||
|
|
||||||
genreDuration := time.Since(genreStart)
|
genreDuration := time.Since(genreStart)
|
||||||
|
|
||||||
|
// Batch-load cover art + MusicBrainz IDs for the matched rows only.
|
||||||
|
// These fields are presentation-only (track-row styling); keeping
|
||||||
|
// them out of the lean query avoids a per-row correlated subquery
|
||||||
|
// and cover-art join over the whole library before WHERE/LIMIT.
|
||||||
|
artStart := time.Now()
|
||||||
|
|
||||||
|
artworkByRecording, err := fetchArtwork(db, recordingIDs)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
for i, rid := range recordingIDs {
|
||||||
|
art, ok := artworkByRecording[rid]
|
||||||
|
if !ok {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
tracks[i].ArtistMBID = art.artistMBID
|
||||||
|
tracks[i].ReleaseGroupMBID = art.releaseGroupMBID
|
||||||
|
tracks[i].RecordingMBID = art.recordingMBID
|
||||||
|
|
||||||
|
if art.coverArtPath != "" {
|
||||||
|
urls := coverart.ResolveURLs(art.coverArtPath)
|
||||||
|
tracks[i].CoverArtPath = urls.Original
|
||||||
|
tracks[i].CoverArtSmall = urls.Small
|
||||||
|
tracks[i].CoverArtMedium = urls.Medium
|
||||||
|
tracks[i].CoverArtLarge = urls.Large
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
artDuration := time.Since(artStart)
|
||||||
|
|
||||||
// Apply genre-sort and deferred LIMIT in Go if needed.
|
// Apply genre-sort and deferred LIMIT in Go if needed.
|
||||||
if sortByGenre {
|
if sortByGenre {
|
||||||
dir := 1
|
dir := 1
|
||||||
@@ -743,6 +762,7 @@ func Evaluate(
|
|||||||
"tracks", len(tracks),
|
"tracks", len(tracks),
|
||||||
"main_ms", mainDuration.Milliseconds(),
|
"main_ms", mainDuration.Milliseconds(),
|
||||||
"genres_ms", genreDuration.Milliseconds(),
|
"genres_ms", genreDuration.Milliseconds(),
|
||||||
|
"artwork_ms", artDuration.Milliseconds(),
|
||||||
"total_ms", time.Since(start).Milliseconds(),
|
"total_ms", time.Since(start).Milliseconds(),
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -778,11 +798,6 @@ func scanTracks(rows *sql.Rows) ([]library.Track, []int64, error) {
|
|||||||
fileSize int64
|
fileSize int64
|
||||||
playCount int64
|
playCount int64
|
||||||
lastPlayed string
|
lastPlayed string
|
||||||
|
|
||||||
coverArtPath string
|
|
||||||
artistMBID string
|
|
||||||
releaseGroupMBID string
|
|
||||||
recordingMBID string
|
|
||||||
)
|
)
|
||||||
|
|
||||||
if err := rows.Scan(
|
if err := rows.Scan(
|
||||||
@@ -792,8 +807,6 @@ func scanTracks(rows *sql.Rows) ([]library.Track, []int64, error) {
|
|||||||
&sampleRate, &bitDepth, &channels,
|
&sampleRate, &bitDepth, &channels,
|
||||||
&bitrate, &fileSize,
|
&bitrate, &fileSize,
|
||||||
&playCount, &lastPlayed,
|
&playCount, &lastPlayed,
|
||||||
&coverArtPath, &artistMBID,
|
|
||||||
&releaseGroupMBID, &recordingMBID,
|
|
||||||
); err != nil {
|
); err != nil {
|
||||||
return nil, nil, fmt.Errorf(
|
return nil, nil, fmt.Errorf(
|
||||||
"could not scan smart playlist row: %w", err,
|
"could not scan smart playlist row: %w", err,
|
||||||
@@ -801,34 +814,23 @@ func scanTracks(rows *sql.Rows) ([]library.Track, []int64, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
track := library.Track{
|
track := library.Track{
|
||||||
TrackName: title,
|
TrackName: title,
|
||||||
ArtistName: artistName,
|
ArtistName: artistName,
|
||||||
TrackLength: strconv.FormatInt(lengthMs, 10),
|
TrackLength: strconv.FormatInt(lengthMs, 10),
|
||||||
FilePath: filePath,
|
FilePath: filePath,
|
||||||
TrackNumber: trackNumber.Int64,
|
TrackNumber: trackNumber.Int64,
|
||||||
DiscNumber: discNumber.Int64,
|
DiscNumber: discNumber.Int64,
|
||||||
Album: album,
|
Album: album,
|
||||||
Year: year,
|
Year: year,
|
||||||
Composer: composer,
|
Composer: composer,
|
||||||
FileType: fileType,
|
FileType: fileType,
|
||||||
SampleRate: sampleRate,
|
SampleRate: sampleRate,
|
||||||
BitDepth: bitDepth,
|
BitDepth: bitDepth,
|
||||||
Channels: channels,
|
Channels: channels,
|
||||||
Bitrate: bitrate,
|
Bitrate: bitrate,
|
||||||
FileSize: fileSize,
|
FileSize: fileSize,
|
||||||
PlayCount: playCount,
|
PlayCount: playCount,
|
||||||
LastPlayed: lastPlayed,
|
LastPlayed: lastPlayed,
|
||||||
ArtistMBID: artistMBID,
|
|
||||||
ReleaseGroupMBID: releaseGroupMBID,
|
|
||||||
RecordingMBID: recordingMBID,
|
|
||||||
}
|
|
||||||
|
|
||||||
if coverArtPath != "" {
|
|
||||||
urls := coverart.ResolveURLs(coverArtPath)
|
|
||||||
track.CoverArtPath = urls.Original
|
|
||||||
track.CoverArtSmall = urls.Small
|
|
||||||
track.CoverArtMedium = urls.Medium
|
|
||||||
track.CoverArtLarge = urls.Large
|
|
||||||
}
|
}
|
||||||
|
|
||||||
tracks = append(tracks, track)
|
tracks = append(tracks, track)
|
||||||
@@ -929,6 +931,128 @@ func fetchGenres(
|
|||||||
return result, nil
|
return result, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// trackArtwork holds the presentation-only cover-art path and
|
||||||
|
// MusicBrainz identifiers attached to a matched track after the main
|
||||||
|
// filter query, keyed by recording_id.
|
||||||
|
type trackArtwork struct {
|
||||||
|
coverArtPath string
|
||||||
|
artistMBID string
|
||||||
|
releaseGroupMBID string
|
||||||
|
recordingMBID string
|
||||||
|
}
|
||||||
|
|
||||||
|
// fetchArtwork batch-loads cover-art paths and MusicBrainz IDs for the
|
||||||
|
// given recording_ids in a single IN-list query. These fields drive
|
||||||
|
// track-row styling only, so scoping them to the matched result set
|
||||||
|
// keeps the cost proportional to results rather than library size.
|
||||||
|
func fetchArtwork(
|
||||||
|
db *database.DB, ids []int64,
|
||||||
|
) (map[int64]trackArtwork, error) {
|
||||||
|
if len(ids) == 0 {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Deduplicate to keep the IN list minimal.
|
||||||
|
seen := make(map[int64]struct{}, len(ids))
|
||||||
|
unique := make([]int64, 0, len(ids))
|
||||||
|
|
||||||
|
for _, id := range ids {
|
||||||
|
if id == 0 {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, ok := seen[id]; ok {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
seen[id] = struct{}{}
|
||||||
|
|
||||||
|
unique = append(unique, id)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(unique) == 0 {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
placeholders := make([]string, len(unique))
|
||||||
|
|
||||||
|
for i := range unique {
|
||||||
|
placeholders[i] = "?"
|
||||||
|
}
|
||||||
|
|
||||||
|
inList := strings.Join(placeholders, ", ")
|
||||||
|
|
||||||
|
// A recording's artist credit can name several artists; the old
|
||||||
|
// correlated subquery picked one via LIMIT 1. GROUP BY r.id with
|
||||||
|
// MIN() reproduces a single stable value without multiplying rows.
|
||||||
|
// SAFETY: placeholders are static "?" tokens; every value is
|
||||||
|
// parameterized. The IN list is bound twice (subquery + outer).
|
||||||
|
query := `SELECT r.id,
|
||||||
|
COALESCE(MIN(ca.file_path), '') AS cover_art_path,
|
||||||
|
COALESCE(MIN(a.mbid), '') AS artist_mbid,
|
||||||
|
COALESCE(MIN(rg.mbid), '') AS release_group_mbid,
|
||||||
|
COALESCE(r.mbid, '') AS recording_mbid
|
||||||
|
FROM recordings r
|
||||||
|
LEFT JOIN artist_credit ac ON r.artist_credit_id = ac.id
|
||||||
|
LEFT JOIN artist_credit_artist aca ON aca.credit_id = ac.id
|
||||||
|
LEFT JOIN artists a ON a.id = aca.artist_id
|
||||||
|
LEFT JOIN (
|
||||||
|
SELECT recording_id,
|
||||||
|
MIN(release_group_id) AS release_group_id
|
||||||
|
FROM release_group_recordings
|
||||||
|
WHERE recording_id IN (` + inList + `)
|
||||||
|
GROUP BY recording_id
|
||||||
|
) rgr ON r.id = rgr.recording_id
|
||||||
|
LEFT JOIN release_groups rg ON rgr.release_group_id = rg.id
|
||||||
|
LEFT JOIN cover_art ca ON rg.cover_art_id = ca.id
|
||||||
|
WHERE r.id IN (` + inList + `)
|
||||||
|
GROUP BY r.id`
|
||||||
|
|
||||||
|
args := make([]any, 0, len(unique)*2)
|
||||||
|
for range 2 {
|
||||||
|
for _, id := range unique {
|
||||||
|
args = append(args, id)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
rows, err := db.QueryContext(query, args...)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf(
|
||||||
|
"smart playlist artwork fetch failed: %w", err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
defer func() { _ = rows.Close() }()
|
||||||
|
|
||||||
|
result := make(map[int64]trackArtwork, len(unique))
|
||||||
|
|
||||||
|
for rows.Next() {
|
||||||
|
var (
|
||||||
|
rid int64
|
||||||
|
art trackArtwork
|
||||||
|
)
|
||||||
|
|
||||||
|
if err := rows.Scan(
|
||||||
|
&rid, &art.coverArtPath, &art.artistMBID,
|
||||||
|
&art.releaseGroupMBID, &art.recordingMBID,
|
||||||
|
); err != nil {
|
||||||
|
return nil, fmt.Errorf(
|
||||||
|
"could not scan smart playlist artwork row: %w", err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
result[rid] = art
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := rows.Err(); err != nil {
|
||||||
|
return nil, fmt.Errorf(
|
||||||
|
"smart playlist artwork iteration error: %w", err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return result, nil
|
||||||
|
}
|
||||||
|
|
||||||
// ParseRuleSet parses a JSON string into a validated RuleSet.
|
// ParseRuleSet parses a JSON string into a validated RuleSet.
|
||||||
func ParseRuleSet(jsonStr string) (RuleSet, error) {
|
func ParseRuleSet(jsonStr string) (RuleSet, error) {
|
||||||
var rs RuleSet
|
var rs RuleSet
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"yellowjacket/backend/coverart"
|
||||||
"yellowjacket/backend/database"
|
"yellowjacket/backend/database"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -812,6 +813,85 @@ func TestEvaluate_TextIs(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestEvaluate_ArtworkEnrichment verifies the presentation-only
|
||||||
|
// cover-art and MusicBrainz-ID fields are attached to matched tracks
|
||||||
|
// by the batched fetchArtwork pass (they are no longer part of the
|
||||||
|
// lean filter query).
|
||||||
|
func TestEvaluate_ArtworkEnrichment(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
db := database.NewTestDB(t)
|
||||||
|
|
||||||
|
// Minimal FK chain: cover_art → release_group(mbid) →
|
||||||
|
// release_group_recordings → recording(mbid) → audio_file, plus
|
||||||
|
// artist_credit → artist_credit_artist → artist(mbid).
|
||||||
|
exec := func(query string, args ...any) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
if _, err := db.ExecContext(query, args...); err != nil {
|
||||||
|
t.Fatalf("seed %q: %v", query, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// file_types are pre-seeded by the schema (id 0 = .mp3).
|
||||||
|
exec("INSERT INTO cover_art (id, file_path, mime_type) " +
|
||||||
|
"VALUES (1, '/covers/abc123.jpg', 'image/jpeg')")
|
||||||
|
exec("INSERT INTO artists (id, name, mbid) " +
|
||||||
|
"VALUES (1, 'Queen', 'artist-mbid-1')")
|
||||||
|
exec("INSERT INTO artist_credit (id, text) VALUES (1, 'Queen')")
|
||||||
|
exec("INSERT INTO artist_credit_artist (credit_id, artist_id) " +
|
||||||
|
"VALUES (1, 1)")
|
||||||
|
exec("INSERT INTO release_groups (id, name, cover_art_id, mbid) " +
|
||||||
|
"VALUES (1, 'A Night at the Opera', 1, 'rg-mbid-1')")
|
||||||
|
exec("INSERT INTO recordings (id, name, artist_credit_id, mbid) " +
|
||||||
|
"VALUES (1, 'Bohemian Rhapsody', 1, 'rec-mbid-1')")
|
||||||
|
exec("INSERT INTO release_group_recordings " +
|
||||||
|
"(release_group_id, recording_id) VALUES (1, 1)")
|
||||||
|
exec("INSERT INTO audio_files (id, file_path, " +
|
||||||
|
"length_milliseconds, recording_id, file_type_id) " +
|
||||||
|
"VALUES (1, '/music/bohemian.mp3', 354000, 1, 0)")
|
||||||
|
|
||||||
|
tracks, err := Evaluate(db, RuleSet{
|
||||||
|
Rules: []Rule{
|
||||||
|
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Evaluate: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(tracks) != 1 {
|
||||||
|
t.Fatalf("got %d tracks, want 1", len(tracks))
|
||||||
|
}
|
||||||
|
|
||||||
|
tr := tracks[0]
|
||||||
|
|
||||||
|
if tr.ArtistMBID != "artist-mbid-1" {
|
||||||
|
t.Errorf("ArtistMBID = %q, want artist-mbid-1", tr.ArtistMBID)
|
||||||
|
}
|
||||||
|
|
||||||
|
if tr.ReleaseGroupMBID != "rg-mbid-1" {
|
||||||
|
t.Errorf("ReleaseGroupMBID = %q, want rg-mbid-1",
|
||||||
|
tr.ReleaseGroupMBID)
|
||||||
|
}
|
||||||
|
|
||||||
|
if tr.RecordingMBID != "rec-mbid-1" {
|
||||||
|
t.Errorf("RecordingMBID = %q, want rec-mbid-1",
|
||||||
|
tr.RecordingMBID)
|
||||||
|
}
|
||||||
|
|
||||||
|
wantURLs := coverart.ResolveURLs("/covers/abc123.jpg")
|
||||||
|
if tr.CoverArtPath != wantURLs.Original {
|
||||||
|
t.Errorf("CoverArtPath = %q, want %q",
|
||||||
|
tr.CoverArtPath, wantURLs.Original)
|
||||||
|
}
|
||||||
|
|
||||||
|
if tr.CoverArtSmall != wantURLs.Small {
|
||||||
|
t.Errorf("CoverArtSmall = %q, want %q",
|
||||||
|
tr.CoverArtSmall, wantURLs.Small)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestEvaluate_TextContains(t *testing.T) {
|
func TestEvaluate_TextContains(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user