fix(image-pipeline): dedupe in-flight preview requests
This commit is contained in:
@@ -3,7 +3,7 @@ import {
|
||||
renderPreview,
|
||||
type PreviewRenderResult,
|
||||
} from "@/lib/image-pipeline/preview-renderer";
|
||||
import type { PipelineStep } from "@/lib/image-pipeline/contracts";
|
||||
import { hashPipeline, type PipelineStep } from "@/lib/image-pipeline/contracts";
|
||||
import type { HistogramData } from "@/lib/image-pipeline/histogram";
|
||||
import type { RenderFullOptions, RenderFullResult } from "@/lib/image-pipeline/render-types";
|
||||
|
||||
@@ -76,6 +76,14 @@ let workerInstance: Worker | null = null;
|
||||
let workerInitError: Error | null = null;
|
||||
let requestIdCounter = 0;
|
||||
const pendingRequests = new Map<number, PendingRequest>();
|
||||
const inFlightPreviewRequests = new Map<string, SharedPreviewRequest>();
|
||||
|
||||
type SharedPreviewRequest = {
|
||||
promise: Promise<PreviewRenderResult>;
|
||||
abortController: AbortController;
|
||||
consumers: Set<symbol>;
|
||||
settled: boolean;
|
||||
};
|
||||
|
||||
function nextRequestId(): number {
|
||||
requestIdCounter += 1;
|
||||
@@ -264,7 +272,20 @@ function runWorkerRequest<TResponse extends PreviewRenderResult | RenderFullResu
|
||||
});
|
||||
}
|
||||
|
||||
export async function renderPreviewWithWorkerFallback(options: {
|
||||
function getPreviewRequestKey(options: {
|
||||
sourceUrl: string;
|
||||
steps: readonly PipelineStep[];
|
||||
previewWidth: number;
|
||||
includeHistogram?: boolean;
|
||||
}): string {
|
||||
return [
|
||||
hashPipeline(options.sourceUrl, options.steps),
|
||||
options.previewWidth,
|
||||
options.includeHistogram === true ? 1 : 0,
|
||||
].join(":");
|
||||
}
|
||||
|
||||
async function runPreviewRequest(options: {
|
||||
sourceUrl: string;
|
||||
steps: readonly PipelineStep[];
|
||||
previewWidth: number;
|
||||
@@ -295,6 +316,105 @@ export async function renderPreviewWithWorkerFallback(options: {
|
||||
}
|
||||
}
|
||||
|
||||
function getOrCreateSharedPreviewRequest(options: {
|
||||
sourceUrl: string;
|
||||
steps: readonly PipelineStep[];
|
||||
previewWidth: number;
|
||||
includeHistogram?: boolean;
|
||||
}): SharedPreviewRequest {
|
||||
const key = getPreviewRequestKey(options);
|
||||
const existing = inFlightPreviewRequests.get(key);
|
||||
if (existing) {
|
||||
return existing;
|
||||
}
|
||||
|
||||
const abortController = new AbortController();
|
||||
const sharedRequest: SharedPreviewRequest = {
|
||||
abortController,
|
||||
consumers: new Set(),
|
||||
settled: false,
|
||||
promise: Promise.resolve(undefined as never),
|
||||
};
|
||||
|
||||
sharedRequest.promise = runPreviewRequest({
|
||||
...options,
|
||||
signal: abortController.signal,
|
||||
}).finally(() => {
|
||||
sharedRequest.settled = true;
|
||||
inFlightPreviewRequests.delete(key);
|
||||
});
|
||||
|
||||
inFlightPreviewRequests.set(key, sharedRequest);
|
||||
return sharedRequest;
|
||||
}
|
||||
|
||||
export async function renderPreviewWithWorkerFallback(options: {
|
||||
sourceUrl: string;
|
||||
steps: readonly PipelineStep[];
|
||||
previewWidth: number;
|
||||
includeHistogram?: boolean;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<PreviewRenderResult> {
|
||||
if (options.signal?.aborted) {
|
||||
throw makeAbortError();
|
||||
}
|
||||
|
||||
const sharedRequest = getOrCreateSharedPreviewRequest({
|
||||
sourceUrl: options.sourceUrl,
|
||||
steps: options.steps,
|
||||
previewWidth: options.previewWidth,
|
||||
includeHistogram: options.includeHistogram,
|
||||
});
|
||||
|
||||
return await new Promise<PreviewRenderResult>((resolve, reject) => {
|
||||
const consumerId = Symbol("preview-consumer");
|
||||
let settled = false;
|
||||
|
||||
const settleOnce = (callback: () => void): void => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
|
||||
settled = true;
|
||||
sharedRequest.consumers.delete(consumerId);
|
||||
if (options.signal) {
|
||||
options.signal.removeEventListener("abort", abortHandler);
|
||||
}
|
||||
|
||||
if (!sharedRequest.settled && sharedRequest.consumers.size === 0) {
|
||||
sharedRequest.abortController.abort();
|
||||
}
|
||||
|
||||
callback();
|
||||
};
|
||||
|
||||
const abortHandler = () => {
|
||||
settleOnce(() => {
|
||||
reject(makeAbortError());
|
||||
});
|
||||
};
|
||||
|
||||
sharedRequest.consumers.add(consumerId);
|
||||
|
||||
if (options.signal) {
|
||||
options.signal.addEventListener("abort", abortHandler, { once: true });
|
||||
}
|
||||
|
||||
sharedRequest.promise.then(
|
||||
(result) => {
|
||||
settleOnce(() => {
|
||||
resolve(result);
|
||||
});
|
||||
},
|
||||
(error: unknown) => {
|
||||
settleOnce(() => {
|
||||
reject(error);
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
export async function renderFullWithWorkerFallback(
|
||||
options: RenderFullOptions,
|
||||
): Promise<RenderFullResult> {
|
||||
|
||||
@@ -210,6 +210,57 @@ describe("usePipelinePreview", () => {
|
||||
);
|
||||
expect(previewHarnessState.latestHistogram).toEqual(histogram);
|
||||
});
|
||||
|
||||
it("restarts preview rendering when the computed preview width changes", async () => {
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
createElement(PreviewHarness, {
|
||||
sourceUrl: "https://cdn.example.com/source.png",
|
||||
steps: [],
|
||||
includeHistogram: false,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(16);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
function WidePreviewHarness() {
|
||||
const { canvasRef } = usePipelinePreview({
|
||||
sourceUrl: "https://cdn.example.com/source.png",
|
||||
steps: [],
|
||||
nodeWidth: 640,
|
||||
includeHistogram: false,
|
||||
});
|
||||
|
||||
return createElement("canvas", { ref: canvasRef });
|
||||
}
|
||||
|
||||
await act(async () => {
|
||||
root?.render(createElement(WidePreviewHarness));
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(16);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(workerClientMocks.renderPreviewWithWorkerFallback).toHaveBeenCalledTimes(2);
|
||||
expect(workerClientMocks.renderPreviewWithWorkerFallback).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({
|
||||
previewWidth: 320,
|
||||
}),
|
||||
);
|
||||
expect(workerClientMocks.renderPreviewWithWorkerFallback).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({
|
||||
previewWidth: 640,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("preview histogram call sites", () => {
|
||||
|
||||
@@ -40,6 +40,10 @@ type WorkerMessage =
|
||||
| {
|
||||
kind: "preview" | "full";
|
||||
requestId: number;
|
||||
payload?: {
|
||||
previewWidth?: number;
|
||||
includeHistogram?: boolean;
|
||||
};
|
||||
}
|
||||
| {
|
||||
kind: "cancel";
|
||||
@@ -65,10 +69,39 @@ class FakeWorker {
|
||||
}
|
||||
}
|
||||
|
||||
function createDeferred<T>() {
|
||||
let resolve!: (value: T | PromiseLike<T>) => void;
|
||||
let reject!: (reason?: unknown) => void;
|
||||
const promise = new Promise<T>((innerResolve, innerReject) => {
|
||||
resolve = innerResolve;
|
||||
reject = innerReject;
|
||||
});
|
||||
|
||||
return {
|
||||
promise,
|
||||
resolve,
|
||||
reject,
|
||||
};
|
||||
}
|
||||
|
||||
describe("worker-client fallbacks", () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
vi.unstubAllGlobals();
|
||||
vi.stubGlobal(
|
||||
"ImageData",
|
||||
class ImageData {
|
||||
data: Uint8ClampedArray;
|
||||
width: number;
|
||||
height: number;
|
||||
|
||||
constructor(data: Uint8ClampedArray, width: number, height: number) {
|
||||
this.data = data;
|
||||
this.width = width;
|
||||
this.height = height;
|
||||
}
|
||||
},
|
||||
);
|
||||
previewRendererMocks.renderPreview.mockReset();
|
||||
bridgeMocks.renderFull.mockReset();
|
||||
previewRendererMocks.renderPreview.mockResolvedValue({
|
||||
@@ -185,4 +218,191 @@ describe("worker-client fallbacks", () => {
|
||||
expect(previewResult.width).toBe(16);
|
||||
expect(fullResult.format).toBe("png");
|
||||
});
|
||||
|
||||
it("shares one worker preview execution across identical requests", async () => {
|
||||
const workerMessages: WorkerMessage[] = [];
|
||||
FakeWorker.behavior = (worker, message) => {
|
||||
workerMessages.push(message);
|
||||
if (message.kind !== "preview") {
|
||||
return;
|
||||
}
|
||||
|
||||
queueMicrotask(() => {
|
||||
worker.onmessage?.({
|
||||
data: {
|
||||
kind: "preview-result",
|
||||
requestId: message.requestId,
|
||||
payload: {
|
||||
width: 8,
|
||||
height: 4,
|
||||
histogram: emptyHistogram(),
|
||||
pixels: new Uint8ClampedArray(8 * 4 * 4).buffer,
|
||||
},
|
||||
},
|
||||
} as MessageEvent);
|
||||
});
|
||||
};
|
||||
vi.stubGlobal("Worker", FakeWorker as unknown as typeof Worker);
|
||||
|
||||
const { renderPreviewWithWorkerFallback } = await import("@/lib/image-pipeline/worker-client");
|
||||
|
||||
const request = {
|
||||
sourceUrl: "https://cdn.example.com/source.png",
|
||||
steps: [],
|
||||
previewWidth: 128,
|
||||
includeHistogram: true,
|
||||
} as const;
|
||||
|
||||
const [first, second] = await Promise.all([
|
||||
renderPreviewWithWorkerFallback(request),
|
||||
renderPreviewWithWorkerFallback(request),
|
||||
]);
|
||||
|
||||
expect(workerMessages.filter((message) => message.kind === "preview")).toHaveLength(1);
|
||||
expect(previewRendererMocks.renderPreview).not.toHaveBeenCalled();
|
||||
expect(first.width).toBe(8);
|
||||
expect(second.width).toBe(8);
|
||||
});
|
||||
|
||||
it("creates separate preview executions when width or histogram settings differ", async () => {
|
||||
const workerMessages: WorkerMessage[] = [];
|
||||
FakeWorker.behavior = (worker, message) => {
|
||||
workerMessages.push(message);
|
||||
if (message.kind !== "preview") {
|
||||
return;
|
||||
}
|
||||
|
||||
queueMicrotask(() => {
|
||||
worker.onmessage?.({
|
||||
data: {
|
||||
kind: "preview-result",
|
||||
requestId: message.requestId,
|
||||
payload: {
|
||||
width: message.payload?.previewWidth ?? 1,
|
||||
height: 4,
|
||||
histogram: emptyHistogram(),
|
||||
pixels: new Uint8ClampedArray((message.payload?.previewWidth ?? 1) * 4 * 4).buffer,
|
||||
},
|
||||
},
|
||||
} as MessageEvent);
|
||||
});
|
||||
};
|
||||
vi.stubGlobal("Worker", FakeWorker as unknown as typeof Worker);
|
||||
|
||||
const { renderPreviewWithWorkerFallback } = await import("@/lib/image-pipeline/worker-client");
|
||||
|
||||
await Promise.all([
|
||||
renderPreviewWithWorkerFallback({
|
||||
sourceUrl: "https://cdn.example.com/source.png",
|
||||
steps: [],
|
||||
previewWidth: 128,
|
||||
includeHistogram: false,
|
||||
}),
|
||||
renderPreviewWithWorkerFallback({
|
||||
sourceUrl: "https://cdn.example.com/source.png",
|
||||
steps: [],
|
||||
previewWidth: 256,
|
||||
includeHistogram: false,
|
||||
}),
|
||||
renderPreviewWithWorkerFallback({
|
||||
sourceUrl: "https://cdn.example.com/source.png",
|
||||
steps: [],
|
||||
previewWidth: 128,
|
||||
includeHistogram: true,
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(workerMessages.filter((message) => message.kind === "preview")).toHaveLength(3);
|
||||
});
|
||||
|
||||
it("removes aborted subscribers without canceling surviving identical preview consumers", async () => {
|
||||
const workerMessages: WorkerMessage[] = [];
|
||||
const previewStarted = createDeferred<void>();
|
||||
|
||||
FakeWorker.behavior = (worker, message) => {
|
||||
workerMessages.push(message);
|
||||
if (message.kind !== "preview") {
|
||||
return;
|
||||
}
|
||||
|
||||
previewStarted.resolve();
|
||||
queueMicrotask(() => {
|
||||
worker.onmessage?.({
|
||||
data: {
|
||||
kind: "preview-result",
|
||||
requestId: message.requestId,
|
||||
payload: {
|
||||
width: 8,
|
||||
height: 4,
|
||||
histogram: emptyHistogram(),
|
||||
pixels: new Uint8ClampedArray(8 * 4 * 4).buffer,
|
||||
},
|
||||
},
|
||||
} as MessageEvent);
|
||||
});
|
||||
};
|
||||
vi.stubGlobal("Worker", FakeWorker as unknown as typeof Worker);
|
||||
|
||||
const { renderPreviewWithWorkerFallback } = await import("@/lib/image-pipeline/worker-client");
|
||||
|
||||
const firstController = new AbortController();
|
||||
const secondController = new AbortController();
|
||||
const request = {
|
||||
sourceUrl: "https://cdn.example.com/source.png",
|
||||
steps: [],
|
||||
previewWidth: 128,
|
||||
includeHistogram: true,
|
||||
} as const;
|
||||
|
||||
const firstPromise = renderPreviewWithWorkerFallback({
|
||||
...request,
|
||||
signal: firstController.signal,
|
||||
});
|
||||
const secondPromise = renderPreviewWithWorkerFallback({
|
||||
...request,
|
||||
signal: secondController.signal,
|
||||
});
|
||||
|
||||
await previewStarted.promise;
|
||||
firstController.abort();
|
||||
|
||||
await expect(firstPromise).rejects.toMatchObject({ name: "AbortError" });
|
||||
await expect(secondPromise).resolves.toMatchObject({ width: 8, height: 4 });
|
||||
expect(workerMessages.filter((message) => message.kind === "preview")).toHaveLength(1);
|
||||
expect(workerMessages.filter((message) => message.kind === "cancel")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("shares one fallback preview execution across identical requests", async () => {
|
||||
vi.stubGlobal("Worker", undefined);
|
||||
const deferred = createDeferred<Awaited<ReturnType<typeof previewRendererMocks.renderPreview>>>();
|
||||
previewRendererMocks.renderPreview.mockReturnValueOnce(deferred.promise);
|
||||
|
||||
const { renderPreviewWithWorkerFallback } = await import("@/lib/image-pipeline/worker-client");
|
||||
|
||||
const firstPromise = renderPreviewWithWorkerFallback({
|
||||
sourceUrl: "https://cdn.example.com/source.png",
|
||||
steps: [],
|
||||
previewWidth: 128,
|
||||
includeHistogram: false,
|
||||
});
|
||||
const secondPromise = renderPreviewWithWorkerFallback({
|
||||
sourceUrl: "https://cdn.example.com/source.png",
|
||||
steps: [],
|
||||
previewWidth: 128,
|
||||
includeHistogram: false,
|
||||
});
|
||||
|
||||
expect(previewRendererMocks.renderPreview).toHaveBeenCalledTimes(1);
|
||||
|
||||
deferred.resolve({
|
||||
width: 16,
|
||||
height: 16,
|
||||
imageData: { data: new Uint8ClampedArray(16 * 16 * 4) },
|
||||
histogram: emptyHistogram(),
|
||||
});
|
||||
|
||||
const [first, second] = await Promise.all([firstPromise, secondPromise]);
|
||||
expect(first.width).toBe(16);
|
||||
expect(second.width).toBe(16);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user