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,