Repository navigation
perf(images): downscale via libvips before decoding for thumbhash - #821
Merged
Merged
Conversation
imageutil.Thumbhash decoded the full original bytes in pure Go before scaling to 100px, materializing the whole raster on the heap — a large provider poster reaches well over a hundred MiB — once per concurrent image-cache worker. The memory-efficient path already existed: normalizeThumbhashSource shrinks to 100px through libvips with shrink-on-load, but only ran as a fallback when the Go decode failed. The order is now reversed: vips downscales first and Go decodes the tiny PNG, with the pure-Go decode of the raw bytes kept as the fallback for anything vips cannot parse. Hashing a 6000x4000 JPEG now allocates about 0.5 MiB on the Go heap instead of over 36 MiB, pinned by a test. The emitted hash bytes change for a given image. Stored thumbhashes remain valid placeholders, and the one equality-comparison site — ebook scan cover change detection (internal/scanner/ebook_scan.go) — stores the freshly computed hash whenever it re-caches, so the change costs one re-cache per scan-covered ebook on its next scan and then converges; it cannot loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Quick104
added a commit
that referenced
this pull request
Aug 29, 2026
…de fallback The vips shrink-on-load rewrite of Thumbhash (#821) removed the header-dimension check this branch had added, leaving the pure-Go decode fallback — the path vips-unparseable bytes take — free to materialize an arbitrarily large raster. Re-apply checkSourceDimensions on the fallback before decoding; TestThumbhashRejectsOversizedHeaderBeforeDecoding pins it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
imageutil.Thumbhashdecodes the full original bytes in pure Go before scaling to 100px. A large provider poster materializes well over a hundred MiB of raster on the Go heap, and this runs once per concurrent image-cache worker — under the raised worker pool from #817, several GiB of transient decode memory in the worst case. The memory-efficient path already existed (normalizeThumbhashSourceshrinks to 100px through libvips with shrink-on-load) but only ran as a fallback when the Go decode failed.Change
The order is reversed: libvips downscales first and Go decodes the resulting ≤100px PNG, with the pure-Go decode of the raw bytes kept as the fallback for anything libvips cannot parse — the supported format set is unchanged. Hashing a 6000×4000 JPEG now allocates ~0.5 MiB on the Go heap instead of 36+ MiB.
Hash-stability analysis (done before changing anything): the emitted hash bytes change for a given image, so every comparison site was audited. All four
Thumbhashcallers were checked (imagecache.CacheBytes, collection artwork, chapter thumbnails, ebook scan); only one compares hashes for equality: ebook scan cover change detection (internal/scanner/ebook_scan.go). That site stores the freshly computed hash whenever it re-caches, so the pipeline change costs one re-cache per scan-covered ebook on its next scan and then converges — it cannot loop. Stored thumbhashes elsewhere are display placeholders and stay valid; nothing needs regeneration.Tests
TestThumbhashDoesNotDecodeFullRasterInGo— pins the property this PR exists for: hashing a 6000×4000 JPEG must not allocate more than 15 MiB on the Go heap (the old path allocates 36+ MiB, the new one ~0.5 MiB).TestThumbhashDeterministic,TestThumbhashRejectsGarbage— the package previously had no tests.BenchmarkThumbhashLargeJPEG— ~9.5 ms/op, ~538 KB/op, 6.7k allocs/op on a 6000×4000 JPEG (Apple M4 Max).Validation
make test-go(CI-equivalent) — passes, including the consumer packages:imagecache,scanner(ebook cover change detection),chapterthumbs,api/handlers,ebooks,audiobooksgo build ./...,go vet,gofmt -lcleangolangci-lint run --new-from-merge-base=origin/main ./...— 0 issuesRelated issue: N/A — narrow fix (flagged during review of #817; internal helper only, no API, client, or jellycompat surface changes)
AI Disclosure
Thumbhashcall site was traced before the change: the ebook scan comparison was confirmed self-healing (it stores the hash it computes on re-cache, so the mismatch fires once per item, not per scan), and the other three sites were confirmed display-only. The memory claim is enforced by a test, not asserted in prose.🤖 Generated with Claude Code