mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
perf(artwork): stop reading and hashing disc art on every request
The resize-cache key was the content hash, which cannot be computed without reading the file, so a warm cache never prevented the I/O: every sized disc request read up to 20MB and hashed it before the lookup. The legacy reader keyed on the id and the album's mtime and touched the file only on a miss. Disc art has no state row and therefore no stored hash, so the key is that same identity — id, album mtime, DiscArtPriority — and the selection chain now runs only when the cache misses. Full-size requests stream the source instead of buffering and hashing it. This matters more now that single-disc albums keep running the disc chain, which puts every disc-tagged track without embedded art on this path.
This commit is contained in:
parent
73519898eb
commit
04d5a55657
@ -303,17 +303,38 @@ func (s *service) serveDisc(ctx context.Context, artID model.ArtworkID, size int
|
||||
}
|
||||
// Single-disc albums run the chain too: a disc can carry its own art, distinct from the
|
||||
// album cover, and DiscArtPriority is what expresses that preference.
|
||||
funcs := dr.fromDiscArtPriority(ctx, s.ffmpeg, conf.Server.DiscArtPriority)
|
||||
if r, path, err := selectImageReader(ctx, artID, funcs...); err == nil && r != nil {
|
||||
defer r.Close()
|
||||
if data, rerr := readCapped(r); rerr == nil {
|
||||
if hash, herr := HashImage(bytes.NewReader(data)); herr == nil {
|
||||
return s.serveBytes(ctx, hash, data, unixMtime(mtimeViaFS(dr.lib.FS, path)), size, square)
|
||||
}
|
||||
}
|
||||
selectImage := func() (io.ReadCloser, string, error) {
|
||||
funcs := dr.fromDiscArtPriority(ctx, s.ffmpeg, conf.Server.DiscArtPriority)
|
||||
return selectImageReader(ctx, artID, funcs...)
|
||||
}
|
||||
albumArtID := model.ArtworkID{Kind: model.KindAlbumArtwork, ID: dr.album.ID}
|
||||
return s.Get(ctx, albumArtID, size, square)
|
||||
if size == 0 && !square {
|
||||
r, path, err := selectImage()
|
||||
if err != nil || r == nil {
|
||||
return s.Get(ctx, albumArtID, size, square)
|
||||
}
|
||||
return &Image{ReadCloser: r, LastUpdated: unixMtime(mtimeViaFS(dr.lib.FS, path))}, nil
|
||||
}
|
||||
|
||||
// Disc art has no state row, so there is no stored content hash to key the resize cache on.
|
||||
// Keying on the id and the album's mtime — as the legacy reader did — lets a warm cache
|
||||
// answer without touching the filesystem at all; the chain runs only on a miss. The key
|
||||
// carries DiscArtPriority so changing it invalidates. resizedItem.hash is key material
|
||||
// here, not a content hash, so the response carries no Image.Hash.
|
||||
key := fmt.Sprintf("%s|%d|%s", artID.ID, dr.album.UpdatedAt.UnixNano(), conf.Server.DiscArtPriority)
|
||||
item := &resizedItem{
|
||||
hash: key,
|
||||
size: size,
|
||||
square: square,
|
||||
lastUpdate: dr.album.UpdatedAt,
|
||||
ffmpeg: s.ffmpeg,
|
||||
open: func() (io.ReadCloser, error) { rc, _, err := selectImage(); return rc, err },
|
||||
}
|
||||
stream, err := s.cache.Get(ctx, item)
|
||||
if err != nil {
|
||||
return s.Get(ctx, albumArtID, size, square)
|
||||
}
|
||||
return &Image{ReadCloser: stream, ETag: representationTag(key, size, square), LastUpdated: dr.album.UpdatedAt}, nil
|
||||
}
|
||||
|
||||
// dangling enqueues a re-resolution at Scan priority and reports the artwork as
|
||||
|
||||
@ -331,6 +331,28 @@ var _ = Describe("Service", func() {
|
||||
Expect(readAll(img)).To(Equal(coverBytes))
|
||||
})
|
||||
|
||||
// The resize cache is keyed on the id and the album's mtime, not on the image bytes, so a
|
||||
// warm hit needs no filesystem access. Dropping the source between the two requests is
|
||||
// how that shows: re-reading would fall back to the album instead.
|
||||
It("serves a sized disc image from cache without re-reading the source", func() {
|
||||
folderRepo.result = []model.Folder{{Path: "tests/fixtures/artist/an-album", ImageFiles: []string{"cover.jpg"}}}
|
||||
albumRepo.SetData(model.Albums{{ID: "aldc3", Name: "Album", FolderIDs: []string{"f1"}}})
|
||||
discID := model.NewArtworkID(model.KindDiscArtwork, model.DiscArtworkID("aldc3", 1), nil)
|
||||
|
||||
first, err := svc.Get(ctx, discID, 64, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
warmed := readAll(first)
|
||||
Expect(warmed).ToNot(BeEmpty())
|
||||
|
||||
// With the source gone and the album holding no art of its own, a re-read would
|
||||
// fall through to the album and fail.
|
||||
folderRepo.result = nil
|
||||
|
||||
second, err := svc.Get(ctx, discID, 64, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(readAll(second)).To(Equal(warmed), "a warm sized request must not touch the source")
|
||||
})
|
||||
|
||||
It("falls back to album art when no disc image matches", func() {
|
||||
folderRepo.result = nil
|
||||
albumRepo.SetData(model.Albums{{ID: "aldc2", Name: "Album"}})
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user