decode once, encode seven times, and one file at a time
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 <noreply@anthropic.com>
This commit is contained in:
+48
-6
@@ -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<Manifest> {
|
||||
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 });
|
||||
|
||||
+11
-5
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user