mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-14 06:16:20 -06:00
* spike: replace sharp with pure-JS image ops (jimp + jsquash WASM) Removes the last native dependency from the ingest path, so the server no longer needs a per-platform/per-ABI prebuilt to thumbnail an image. Motivated by getting the server onto hardware with no toolchain, but the ABI tax is paid on every install — it is the same failure class lib/preflight-deps.js exists to explain. lib/image-ops.js is the whole surface: metadata() and writeThumbnail(), which are the only two things ingest ever asked sharp for. Format parity holds. jpeg/png/gif/tiff/bmp are native to Jimp; webp and avif go through @jsquash WASM, whose bundled .wasm must be compiled by hand because the packages locate it with fetch(file://) and Node has no file:// fetch — the only symptom otherwise is a bare "fetch failed". heic is unsupported, as it already was: sharp advertises heif but its prebuilt libvips refuses HEVC. #170 is preserved by a different mechanism. Jimp applies EXIF orientation at decode and rewrites the tag to 1, so metadata() reports display dimensions and imageDisplayDims() runs as a no-op instead of swapping W/H a second time. The helper stays in the path so the rule keeps living in one place. Verified: 1643/1643 tests pass, and ingest was exercised in a child process with node_modules/sharp moved aside — jpeg, EXIF-rotated jpeg, png, webp, avif, gif all measured and thumbnailed correctly, corrupt input still yields nulls with no phantom thumbnail_path. KNOWN BLOCKER, do not ship as-is: Jimp is pure JS on the main thread, where sharp handed work to a libvips threadpool. A 12MP photo goes 65ms -> 1079ms, and the event loop stalls for 1003ms of it (sharp: zero stalls). thumbnail-backfill.js walks a whole library at boot, so this reproduces #240 exactly — blocked loop, missed heartbeats, panels marked offline, reconnect churn. Needs a worker_thread offload before this is viable; image-ops.js is the seam for it. * Run image decoding on a worker thread Fixes the blocker the previous commit shipped with. Pure-JS decoding costs ~1s of solid CPU for a 12MP photo, and in-process that is not a slow upload but a stalled event loop — no heartbeats, no socket traffic. thumbnail-backfill.js walks a whole library at boot, so it reproduced #240 (blocked loop -> missed heartbeats -> panels offline -> reconnect churn) from our own maintenance. sharp never did this because libvips works on a threadpool. image-ops.js is now a dispatcher over image-ops-worker.js; the work moved unchanged to image-ops-core.js, so callers and their failure contract are untouched. Measured on a 12MP photo: 1079ms wall with the loop stalled 1003ms, to 1881ms wall for two ops with ZERO stalls and 185 timer ticks serviced. Wall time is worse and that is fine — it is off the main thread now. Design notes, all load-bearing: - ONE JOB AT A TIME. A decoded 12MP bitmap is ~48MB of RGBA; overlapping jobs multiply peak memory by queue depth, which is the wrong failure on the small targets this change exists to reach. Costs no throughput — the work is CPU-bound and one busy worker already saturates its core. - unref'd while idle, ref'd only in flight. Otherwise scripts/backfill-rotation- dims.js never exits and `node --test` hangs forever. Verified: a CLI-style run exits in 104ms, code 0. - decode failures reply as messages, so one bad upload cannot tear down the worker and take unrelated queued jobs with it. - in-process fallback if a thread cannot be had, warned rather than silent. test/image-ops.test.js pins the loop-liveness property, which no functional test would catch. Its thresholds were mutation-tested against the inline path: the first version passed there too (4MP stalls only ~355ms, under a non-flaky threshold), so the fixture is 12MP and the thresholds sit in the gap between the two behaviours — worker ~90 ticks/~0ms, inline ~3 ticks/~897ms. It now fails inline, as a guard must. 1647/1647 pass. Ingest re-verified with node_modules/sharp moved aside. * Measure and thumbnail an image from a single decode Ingest asked for metadata() then writeThumbnail(), which decoded the file twice. That pairing was free under sharp, whose .metadata() only parses the header, but every decode here is a full one — ~1s for a 12MP photo — so the naive translation doubled the most expensive thing on the ingest path. image-ops.measureAndThumbnail() returns both from one decode. Full ingest of a 12MP photo: 2 decodes/~1.9s -> 1150ms, still with zero event-loop stalls. The subtlety is the failure contract. In the two-call version width and height were assigned BEFORE the thumbnail was attempted, so a failed thumbnail still left usable dimensions on the row — the player needs them to letterbox. Merging naively would have turned any thumbnail failure into total metadata loss. So a WRITE failure is reported ({thumbnailWritten:false, thumbnailError}) with the dimensions intact, and the caller sets thumbnail_path only when the write succeeded, keeping the phantom-path discipline. A DECODE failure still throws — there is nothing to report about an unreadable image. backfill-rotation-dims.js deliberately keeps the separate calls: it probes every image row but regenerates a thumbnail only for the few whose dimensions changed, so pairing them there would decode files it has no reason to thumbnail. Tests count decodes rather than timing them — an exact property, and a wall-clock comparison would be flaky under load. The count filters for reads of the file under test: Node's ESM loader also goes through fs.promises.readFile, so a raw call count picks up jimp's and the WASM codecs' lazy loading and reads 30 instead of 1. 1649/1649 pass. Ingest re-verified across all 7 formats with sharp moved aside. * Dockerfile: sharp is no longer a production dependency --omit=dev now leaves it out entirely; better-sqlite3 is the only native module the builder stage still needs a toolchain for.
133 lines
7.7 KiB
JavaScript
133 lines
7.7 KiB
JavaScript
'use strict';
|
|
|
|
// Image decoding is pure JavaScript now (no native sharp), so its CPU cost lands on whichever
|
|
// thread runs it — ~1s of solid work for a 12MP photo. In-process that is a stalled event loop:
|
|
// no heartbeats, no socket traffic, panels marked offline, reconnect churn — #240 arriving from
|
|
// our own thumbnail backfill. lib/image-ops therefore hosts the work on a worker thread, and
|
|
// these bites pin the properties that makes it safe, none of which a functional test would catch.
|
|
|
|
const { test, after, mock } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('node:fs');
|
|
const os = require('node:os');
|
|
const path = require('node:path');
|
|
const sharp = require('sharp'); // devDependency: fixture generator only, never shipped
|
|
const imageOps = require('../lib/image-ops');
|
|
|
|
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'image-ops-'));
|
|
after(async () => { await imageOps.shutdown(); fs.rmSync(tmp, { recursive: true, force: true }); });
|
|
|
|
// 12MP — a phone photo, and the size the thresholds below are calibrated against. Smaller is
|
|
// tempting for test speed but defeats the point: at 4MP the inline path stalls only ~350ms, which
|
|
// slips under any threshold loose enough not to be flaky, so the guard stops detecting the very
|
|
// regression it exists for. Measured: inline ~1000ms stall / ~2 timers serviced, worker ~0ms / ~90.
|
|
async function bigPhoto(name = 'big.jpg') {
|
|
const p = path.join(tmp, name);
|
|
if (!fs.existsSync(p)) {
|
|
// Random pixels, not a flat fill: a solid colour compresses to almost nothing and decodes far
|
|
// faster than any real photo, which would quietly defeat the timing assertion below.
|
|
const px = Buffer.allocUnsafe(4000 * 3000 * 3);
|
|
for (let i = 0; i < px.length; i++) px[i] = (i * 2654435761) & 0xff;
|
|
fs.writeFileSync(p, await sharp(px, { raw: { width: 4000, height: 3000, channels: 3 } }).jpeg().toBuffer());
|
|
}
|
|
return p;
|
|
}
|
|
|
|
test('image work does not stall the event loop (#240)', async () => {
|
|
const src = await bigPhoto();
|
|
|
|
let ticks = 0, worstGap = 0, last = Date.now();
|
|
const timer = setInterval(() => { ticks++; worstGap = Math.max(worstGap, Date.now() - last - 10); last = Date.now(); }, 10);
|
|
const started = Date.now();
|
|
await imageOps.writeThumbnail(src, path.join(tmp, 'thumb.jpg'), 320, 70);
|
|
const elapsed = Date.now() - started;
|
|
clearInterval(timer);
|
|
|
|
// The point is not that it was fast — it is that the loop kept running while it was slow.
|
|
// Thresholds sit in the gap between the two behaviours (worker ~90 ticks / ~0ms stall, inline
|
|
// ~2 ticks / ~1000ms stall), far enough from both to bite without being flaky.
|
|
assert.ok(ticks >= 20, `event loop serviced only ${ticks} timers in ${elapsed}ms — it is being blocked`);
|
|
assert.ok(worstGap < 200, `event loop stalled ${worstGap}ms in one go — image work is on the main thread`);
|
|
assert.ok(fs.existsSync(path.join(tmp, 'thumb.jpg')), 'thumbnail was still written');
|
|
});
|
|
|
|
test('an undecodable image rejects without killing the worker', async () => {
|
|
const bad = path.join(tmp, 'corrupt.jpg');
|
|
fs.writeFileSync(bad, Buffer.from('not an image'));
|
|
await assert.rejects(() => imageOps.metadata(bad), 'corrupt input must reject, so ingest records nulls');
|
|
|
|
// Crash isolation: one bad upload must not take out the queued work of unrelated callers.
|
|
const ok = path.join(tmp, 'fine.png');
|
|
fs.writeFileSync(ok, await sharp({ create: { width: 40, height: 25, channels: 3, background: '#123456' } }).png().toBuffer());
|
|
assert.deepEqual(await imageOps.metadata(ok), { width: 40, height: 25, orientation: 1 });
|
|
});
|
|
|
|
test('concurrent callers are serialized, and each still gets its own answer', async () => {
|
|
// Serialization bounds peak memory to ONE decoded bitmap (a 12MP photo is ~48MB of RGBA);
|
|
// overlapping jobs would multiply that by the queue depth on exactly the small targets this
|
|
// change exists to reach. Correctness under concurrency is what is asserted here.
|
|
const sizes = [[30, 10], [60, 20], [90, 30], [120, 40]];
|
|
const files = await Promise.all(sizes.map(async ([w, h], i) => {
|
|
const p = path.join(tmp, `c${i}.png`);
|
|
fs.writeFileSync(p, await sharp({ create: { width: w, height: h, channels: 3, background: '#0a0' } }).png().toBuffer());
|
|
return p;
|
|
}));
|
|
const got = await Promise.all(files.map(f => imageOps.metadata(f)));
|
|
assert.deepEqual(got.map(m => [m.width, m.height]), sizes, 'replies must not be crossed between queued jobs');
|
|
});
|
|
|
|
test('measureAndThumbnail decodes the file exactly once', async () => {
|
|
// Counted, not timed: a wall-clock comparison against metadata()+writeThumbnail() would be
|
|
// flaky under load, and this is an exact property. Asserted against image-ops-core directly
|
|
// because the decode happens on the worker thread, out of reach of a spy set up here.
|
|
// readImage() is the only reader in core, so readFile calls == decodes.
|
|
const core = require('../lib/image-ops-core');
|
|
const fsp = require('node:fs/promises');
|
|
const src = path.join(tmp, 'once.png');
|
|
fs.writeFileSync(src, await sharp({ create: { width: 200, height: 80, channels: 3, background: '#246' } }).png().toBuffer());
|
|
|
|
const spy = mock.method(fsp, 'readFile');
|
|
// Count reads OF THIS FILE only. Node's ESM loader also reads through fs.promises.readFile, so
|
|
// a raw call count picks up jimp's and the WASM codecs' lazy module loading on first use.
|
|
const decodes = () => spy.mock.calls.filter(c => String(c.arguments[0]) === src).length;
|
|
try {
|
|
const r = await core.measureAndThumbnail(src, path.join(tmp, 'once-thumb.jpg'), 100, 70);
|
|
assert.equal(decodes(), 1, 'combined op must decode once, not once per answer');
|
|
assert.deepEqual([r.width, r.height], [200, 80]);
|
|
assert.equal(r.thumbnailWritten, true);
|
|
|
|
// The pairing it replaces, for contrast — this is the cost being removed.
|
|
spy.mock.resetCalls();
|
|
await core.metadata(src);
|
|
await core.writeThumbnail(src, path.join(tmp, 'twice-thumb.jpg'), 100, 70);
|
|
assert.equal(decodes(), 2, 'the separate calls are what cost two decodes');
|
|
} finally { spy.mock.restore(); }
|
|
});
|
|
|
|
test('a thumbnail that cannot be written still yields dimensions', async () => {
|
|
// Dimensions are independently useful — the player needs them to letterbox — and the two-call
|
|
// version kept them, because width/height were assigned before the thumbnail was attempted.
|
|
// Merging the calls must not quietly turn a thumbnail failure into a total metadata failure.
|
|
const src = path.join(tmp, 'ok.png');
|
|
fs.writeFileSync(src, await sharp({ create: { width: 150, height: 60, channels: 3, background: '#654' } }).png().toBuffer());
|
|
|
|
const undirectable = path.join(tmp, 'no-such-dir', 'thumb.jpg'); // parent does not exist
|
|
const r = await imageOps.measureAndThumbnail(src, undirectable, 100, 70);
|
|
assert.deepEqual([r.width, r.height], [150, 60], 'dimensions survive a thumbnail write failure');
|
|
assert.equal(r.thumbnailWritten, false);
|
|
assert.match(r.thumbnailError || '', /ENOENT|no such file/i);
|
|
});
|
|
|
|
test('#170 EXIF orientation is applied by the decoder, so dimensions are as DISPLAYED', async () => {
|
|
// orientation 6 = "rotate 90° CW to display": a 30x100 stored buffer DISPLAYS as 100x30.
|
|
const p = path.join(tmp, 'rot6.jpg');
|
|
fs.writeFileSync(p, await sharp({ create: { width: 30, height: 100, channels: 3, background: '#00ff00' } })
|
|
.withMetadata({ orientation: 6 }).jpeg().toBuffer());
|
|
|
|
const meta = await imageOps.metadata(p);
|
|
assert.equal(meta.width, 100, 'EXIF-rotated image measures as displayed, not as stored');
|
|
assert.equal(meta.height, 30);
|
|
// Reported as 1 because the rotation is already applied — imageDisplayDims() must NOT swap again.
|
|
assert.equal(meta.orientation, 1, 'a tag of 6 here would double-rotate downstream');
|
|
});
|