Skip to content

perf(producer): cache transfer-converted hdr image buffers per render job#384

Open
vanceingalls wants to merge 1 commit intovance/logger-level-gatingfrom
vance/hdr-image-transfer-cache
Open

perf(producer): cache transfer-converted hdr image buffers per render job#384
vanceingalls wants to merge 1 commit intovance/logger-level-gatingfrom
vance/hdr-image-transfer-cache

Conversation

@vanceingalls
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls commented Apr 21, 2026

Summary

Add HdrImageTransferCache — a per-render-job bounded LRU keyed by (imageId, targetTransfer) — so static HDR image layers whose source transfer differs from the render's effective transfer (PQ↔HLG) are converted once per job instead of once per composited frame.

Why

Chunk 8B of plans/hdr-followups.md. blitHdrImageLayer was running Buffer.from + convertTransfer on every composited frame, even though the converted buffer is identical for the entire job. For a multi-second comp at 30 fps this is hundreds of redundant transfer conversions on the hot path.

What changed

  • New packages/producer/src/services/hdrImageTransferCache.ts — bounded LRU keyed by (imageId, targetTransfer) that owns the converted HDR rgb48 buffer for static HDR image layers:
    • Same-transfer requests return the source buffer untouched (zero copy).
    • Cross-transfer requests pay one Buffer.from + convertTransfer on first miss, reuse the cached copy on every subsequent frame.
  • Wired into renderOrchestrator.ts via HdrCompositeContext.hdrImageTransferCache, instantiated once per render job, and consumed by blitHdrImageLayer on both the main composite path and the transition path.

Test plan

  • packages/producer/src/services/hdrImageTransferCache.test.ts — 12 tests:
    • hit/miss semantics
    • distinct keys per image and per target transfer
    • LRU eviction + promotion
    • maxEntries=0 passthrough
    • source-buffer immutability for cached entries
    • invalid options
  • Re-ran the Chunk 8A HDR benchmark — for the hdr-regression fixture (which has cross-transfer image layers) the cache hits 100% after the first frame; for HDR fixtures without cross-transfer images the same-transfer passthrough is a no-op.

Stack

Chunk 8B of plans/hdr-followups.md. Sits on top of Chunk 8C (logger gating) and Chunk 8A (benchmark harness) so the win is measurable.

Copy link
Copy Markdown
Collaborator Author

vanceingalls commented Apr 21, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch from 092e9ba to fdbbfd2 Compare April 22, 2026 03:34
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch from 2365f04 to 442f167 Compare April 22, 2026 03:34
@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch from fdbbfd2 to 3f90688 Compare April 22, 2026 04:46
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch from 442f167 to a746bb2 Compare April 22, 2026 04:46
@vanceingalls
Copy link
Copy Markdown
Collaborator Author

Thanks for the careful review @jrusso1020 — confirming each point you raised landed in the merged commit:

1. Per-render-job scoping (no cross-job collisions). The cache is constructed once per render job inside the orchestrator and goes out of scope on job exit:

          // Per-job LRU cache for transfer-converted HDR image buffers. Static HDR
          // images that need PQ↔HLG conversion are converted exactly once per
          // (imageId, targetTransfer) and then reused for every subsequent frame
          // instead of paying a fresh `Buffer.from` + `convertTransfer` on every
          // composite. The cache is local to this render job so concurrent renders
          // do not share state.
          const hdrImageTransferCache = createHdrImageTransferCache();
          const hdrCompositeCtx: HdrCompositeContext = {
            ...
            hdrImageTransferCache,
            ...
          };

The HdrCompositeContext carries it through blitHdrImageLayer for both the main composite path and the transition path (the two getConverted call sites at lines 554, 718, and 2040).

2. Same-transfer zero-copy. When sourceTransfer === targetTransfer, getConverted returns source untouched — no allocation, no convertTransfer call, no cache write. The orchestrator also skips even calling getConverted when transfer info is unavailable, so the SDR/no-transfer path stays exactly as it was.

3. Source-buffer immutability. convertTransfer mutates in place, so the implementation always clones via Buffer.from(source) before converting on a miss — both the normal LRU path and the maxEntries=0 passthrough. Three of the 15 tests in hdrImageTransferCache.test.ts pin this invariant against future refactors:

  • does not mutate the source buffer on a convert+cache miss
  • does not mutate the source buffer on a convert+cache miss with maxEntries=0 passthrough
  • does not mutate the source buffer on a cache hit

The full 15-test suite also covers hit/miss, distinct keys per (imageId, targetTransfer), LRU eviction + MRU promotion, maxEntries=0 disabling caching while still returning correct bytes, the default cap of 16, and validation that maxEntries rejects non-integers/negatives.

On metrics emission for hit/miss/eviction (non-blocking observation): Agreed it's worth having when we wire up benchmark observability more broadly — leaving as a future follow-up alongside the existing frameDirMaxIndexCache eviction metric in plans/hdr-followups.md. Not added in this PR to keep the change surgical and the cache implementation dependency-free of the logger interface.

@qodo-ai-reviewer
Copy link
Copy Markdown

Hi, createHdrImageTransferCache defaults to maxEntries=16, which can retain very large rgb48le buffers at higher resolutions (e.g., 4K ~53MB per entry → ~850MB worst-case), increasing peak RSS and OOM risk unless callers tune maxEntries.

Severity: remediation recommended | Category: performance

How to fix: Make cache bound configurable

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

Default maxEntries=16 can retain hundreds of MB to ~1GB at 4K+, which can cause memory pressure/OOM on constrained machines.

Issue Context

Cache buffers are full-frame rgb48le images (width*height*6). Count-based LRU doesn’t account for resolution.

Fix Focus Areas

  • packages/producer/src/services/hdrImageTransferCache.ts[74-90]
  • packages/producer/src/services/renderOrchestrator.ts[1903-1921]

Suggested fix

  • Plumb a configurable maxEntries from render/job config or an env var.
  • Optionally set a smaller default when width*height exceeds a threshold (e.g., 4K), or switch to an approximate byte-budget eviction policy.
  • Update docs/comments to mention 4K implications and how to tune.

Found by Qodo code review. FYI, Qodo is free for open-source.

@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch from 3f90688 to bd86b42 Compare April 22, 2026 18:01
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch from a746bb2 to 15eed9b Compare April 22, 2026 18:01
@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch from bd86b42 to a549e46 Compare April 22, 2026 18:35
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch 2 times, most recently from 08c2296 to 8b63af3 Compare April 22, 2026 18:50
@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch from a549e46 to 8d9641b Compare April 22, 2026 18:50
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch from 8b63af3 to 69af136 Compare April 22, 2026 18:59
@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch 2 times, most recently from 3fcaa60 to e675bd9 Compare April 22, 2026 19:34
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch from 69af136 to c72eb7f Compare April 22, 2026 19:34
@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch from e675bd9 to f73328c Compare April 22, 2026 20:45
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch 2 times, most recently from 54c8e1f to b71cf6f Compare April 22, 2026 20:48
@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch 2 times, most recently from 55a41a8 to 349f0f6 Compare April 22, 2026 22:13
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch from b71cf6f to 3c4754f Compare April 22, 2026 22:13
@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch from 349f0f6 to cd65ab0 Compare April 22, 2026 22:54
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch 2 times, most recently from e49a01f to a801330 Compare April 22, 2026 23:26
@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch from 5409b1a to c6a8280 Compare April 23, 2026 00:07
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch from a801330 to 2af81de Compare April 23, 2026 00:08
@vanceingalls vanceingalls force-pushed the vance/logger-level-gating branch from c6a8280 to 2ce89a5 Compare April 23, 2026 00:12
… job

Static HDR image layers whose source transfer differs from the render's effective
transfer (PQ↔HLG) were re-running `Buffer.from` + `convertTransfer` on every
composited frame, even though the converted buffer is identical for the entire
job. Added `HdrImageTransferCache` — a per-render-job bounded LRU keyed by
`(imageId, targetTransfer)` that converts once and reuses on every subsequent
frame, while leaving same-transfer requests as a zero-copy passthrough.

Wired into `renderOrchestrator.ts` via `HdrCompositeContext.hdrImageTransferCache`,
instantiated once per job and consumed by `blitHdrImageLayer` on both the main
composite path and the transition path. Covered by `hdrImageTransferCache.test.ts`
(hit/miss, distinct keys per image and per target transfer, LRU eviction +
promotion, `maxEntries=0` passthrough, source-buffer immutability for cached
entries, invalid options).

Made-with: Cursor
@vanceingalls vanceingalls force-pushed the vance/hdr-image-transfer-cache branch from 2af81de to a3d4984 Compare April 23, 2026 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants