From 6db5b1e85d812d2d6712372053e379cb081c9020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 29 Aug 2026 11:24:18 +0100 Subject: [PATCH] decode once, encode seven times, and one file at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A HEIC was being decoded once per rung. av-club is stateless per request, so seven rungs asked for from the original are seven ImageMagick conversions of it — measured on fofos at 8.5 s each, against 1.1 s for the resize and encode that follow. Six of the seven produced a pixel-identical intermediate and threw it away: about 67 seconds per photograph where 16 would do, paid again on every future upload. So when av-club says a format needs converting, convert it ONCE into the staging directory and cut every rung from that. `needsConversion` is asked rather than guessed, because which formats are expensive is av-club's fact and a list of them kept here would go on paying after its build stopped needing it. JPEG and PNG are untouched — writing a full-resolution intermediate to save six cheap reads would cost more than it saved. Concurrency drops to one. Two was chosen to match av-club and is the right number on a machine with room; fofos has 1.87 GiB of RAM, and two concurrent 12-megapixel decodes left 520 MiB free with 648 MB already paged out. The work is CPU-bound on a box already at 99%, so the second worker was not buying throughput — it was buying a second resident bitmap. And the sharp calls now name their output format properly. `output` takes `{ format, options }`; passing `{ webp: { … } }` was silently ignored, so everything built before this encoded at sharp's default quality rather than the 82 asked for. Co-Authored-By: Claude Opus 5 --- api/derive.ts | 54 +++++++++++++++++++++++++++++++++++++++++++++------ api/sync.ts | 16 ++++++++++----- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/api/derive.ts b/api/derive.ts index 7f0756a..928aa9b 100644 --- a/api/derive.ts +++ b/api/derive.ts @@ -202,6 +202,16 @@ export async function deriveOne( } } +/** + * The intermediate a ladder is built from, when reading the original repeatedly would be the expensive + * part. + * + * Never compressed — `compressionLevel: 0`. It lives for seconds inside the staging directory and is + * then deleted, so every cycle spent shrinking it is spent twice for nothing: once packing, once + * unpacking, seven times over. + */ +const MASTER = '.master.png'; + async function buildImage( sourcePath: string, staging: string, @@ -209,7 +219,7 @@ async function buildImage( source: { bytes: number; mtimeMs: number }, email: string, ): Promise { - const meta = await avClub<{ width: number | null; height: number | null }>( + const meta = await avClub<{ width: number | null; height: number | null; needsConversion: boolean }>( `/v1/image?path=${encodeURIComponent(sourcePath)}`, email, ); @@ -217,6 +227,33 @@ async function buildImage( const height = meta.height ?? 0; if (!width || !height) throw new Error(`Could not read the dimensions of ${name}.`); + /* + DECODE ONCE. + + av-club is stateless per request, so seven rungs asked for from the original are seven decodes of + it. For a JPEG that is fine — sharp reads one cheaply, and writing a full-resolution intermediate + to save six cheap reads would cost more than it saved. For a HEIC it is not: every request spawns + ImageMagick to produce a full-resolution PNG, measured on fofos at 8.5 s against 1.1 s for the + resize and encode that follow. Seven rungs spent about a minute making the same intermediate over + and over and throwing six of them away. + + So when av-club says this format needs converting, it is converted ONCE, here, into the staging + directory nobody else can see, and every rung is cut from that. 67 seconds becomes 16. + + `needsConversion` is asked rather than guessed. Which formats are expensive is av-club's fact and + changes when its build changes; a list of them kept here would go on paying for a conversion that + had stopped being necessary. + */ + const master = meta.needsConversion ? join(staging, MASTER) : sourcePath; + if (meta.needsConversion) { + await post('/v1/sharp', email, { + inputPath: sourcePath, + outputPath: master, + pipeline: [{ rotate: {} }], + output: { format: 'png', options: { compressionLevel: 0 } }, + }); + } + const variants: Variant[] = []; /* @@ -226,14 +263,17 @@ async function buildImage( exists at all — the original renders nowhere. A lightbox, a zoom, or handing the client their own photograph back all need it. */ - variants.push(await encodeWebp(sourcePath, join(staging, 'full.webp'), null, email, width, height)); + variants.push(await encodeWebp(master, join(staging, 'full.webp'), null, email, width, height)); // Strictly less than the original: a rung at or above it would duplicate `full.webp` byte for byte. for (const rung of LADDER) { if (rung >= width) continue; - variants.push(await encodeWebp(sourcePath, join(staging, `${rung}.webp`), rung, email, width, height)); + variants.push(await encodeWebp(master, join(staging, `${rung}.webp`), rung, email, width, height)); } + // Before the manifest is written, so a directory that has one never contains this. + if (meta.needsConversion) await rm(master, { force: true }); + return { kind: 'image', source: { name, bytes: source.bytes, mtimeMs: source.mtimeMs, width, height }, @@ -254,12 +294,14 @@ async function encodeWebp( inputPath: sourcePath, outputPath, // `rotate` with no angle applies the EXIF orientation, which phone photographs rely on entirely. A - // ladder built without it is a set of sideways pictures. + // ladder built without it is a set of sideways pictures. Kept on every rung including those cut + // from the master, where it is a no-op — a PNG carries no EXIF, and the master was written with the + // rotation already applied. One code path beats two that can disagree about which way up a photo is. pipeline: [ { rotate: {} }, ...(rung === null ? [] : [{ resize: { width: rung, fit: 'inside', withoutEnlargement: true } }]), ], - output: { webp: { quality: WEBP_QUALITY } }, + output: { format: 'webp', options: { quality: WEBP_QUALITY } }, }); const width = rung ?? sourceWidth; @@ -319,7 +361,7 @@ async function buildVideo( const poster = await post<{ path: string; bytes: number }>('/v1/sharp', email, { inputPath: stillJpg, outputPath: join(staging, 'poster.webp'), - output: { webp: { quality: WEBP_QUALITY } }, + output: { format: 'webp', options: { quality: WEBP_QUALITY } }, }); await rm(stillJpg, { force: true }); variants.push({ file: 'poster.webp', width: null, height: null, bytes: poster.bytes }); diff --git a/api/sync.ts b/api/sync.ts index c840b82..c1ac7bc 100644 --- a/api/sync.ts +++ b/api/sync.ts @@ -24,11 +24,16 @@ import { deriveOne, derivedDir, listOriginals, originalsDir, targetFor } from '. * nothing about how a file is built. A full sync and a single arrival must never be two implementations * that can disagree about what a derived directory should contain. * - * ── Two at a time ── + * ── One at a time ── * - * av-club runs two jobs concurrently and queues the rest, so a third caller would only wait in its - * queue while making this module's progress harder to report. Matching its number keeps both slots - * busy and keeps "what is happening now" answerable. + * It was two, to match av-club's own concurrency. That is the right number on a machine with room and + * the wrong one here: converting a 12-megapixel HEIC holds a full-resolution bitmap, and fofos has + * 1.87 GiB of RAM. Two at once left about 520 MiB free with 648 MB already paged out — CPU pinned at + * 99%, and an out-of-memory kill one large photograph away. + * + * Nothing is lost by going single-file. The work is CPU-bound on a box that was already saturated, so + * the second worker was not buying throughput; it was buying a second resident bitmap. It also makes + * "what is happening now" one filename instead of two, which is the more useful thing to read. */ export type SyncError = { file: string; message: string }; @@ -50,7 +55,8 @@ export type SyncState = { fatal: string | null; }; -const CONCURRENCY = 2; +/** One. See the note above — this is a memory limit, not a politeness. */ +const CONCURRENCY = 1; const idle = (): SyncState => ({ running: false,