Skip to content

perf(engine): consolidate cache key + reuse Phase 2 metadata + cleanups#437

Closed
jrusso1020 wants to merge 1 commit intoperf/producer-gated-hwaccel-decodefrom
perf/producer-stack-cleanup
Closed

perf(engine): consolidate cache key + reuse Phase 2 metadata + cleanups#437
jrusso1020 wants to merge 1 commit intoperf/producer-gated-hwaccel-decodefrom
perf/producer-stack-cleanup

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 commented Apr 23, 2026

What

Post-stack cleanup based on /simplify review of PRs #430-#435.

Why

Three concrete issues two reviewers independently flagged:

  1. Cache key computed twice per cache missprobeSourceForCacheKey (statSync) + computeExtractionCacheKey (sha256) ran once for lookup and again for markCacheEntryComplete. Hot-path waste in the same PR that adds the cache.
  2. Redundant ffprobe on cache hit — Phase 3 was calling extractVideoMetadata solely to populate ExtractedFrames.metadata, even though Phase 2 already probed the same input. ~30-100ms of unnecessary process spawn per cache hit, in the path that's supposed to be near-instant.
  3. "frame_" filename prefix shared between extractor write and cache lookup with no constant — a one-sided rename would silently produce zero cache hits with no test failure.

Plus two minor cleanups:

  1. existsSync precheck before mkdirSync({recursive: true}) in ensureCacheEntryDir — the recursive mkdir is already idempotent.
  2. sequentialInjectorUsed boolean was redundant with the injector stats counters — the same signal can be derived from hits + misses + inFlightCoalesced > 0.

How

FRAME_FILENAME_PREFIX constant

Exported from extractionCache.ts. Both the extractor's write pattern (${FRAME_FILENAME_PREFIX}%05d.${format}) and the cache lookup's filter (f.startsWith(FRAME_FILENAME_PREFIX)) now reference it.

Phase 2 metadata reuse

Phase 2's color-space probe used to return a 2-field {colorSpace, durationSeconds} shape. Now it stashes the full VideoMetadata directly onto each resolvedVideos[i] as sourceMetadata. Phase 3's cache-hit branch uses sourceMetadata instead of probing again.

This is safe because Phase 3 cache hits can only occur for inputs that did NOT go through HDR or VFR preflight (those mutate videoPath, which changes the cache key), so sourceMetadata is current for cache hits.

Phase 2b VFR detection still re-probes — HDR preflight may have rewritten the file with different isVFR characteristics.

Cache key hoist

Computed once at the top of the per-input map callback and reused for both lookupCacheEntry and markCacheEntryComplete. Eliminates one statSync + one sha256 per cache miss.

sequentialInjectorUsed removal

// before
let sequentialInjectorUsed = false;
if (injector) sequentialInjectorUsed = true;  // ×3
injectorStats: sequentialInjectorUsed ? { ...stats } : undefined

// after
injectorStats:
  injectorCacheStats.hits + injectorCacheStats.misses + injectorCacheStats.inFlightCoalesced > 0
    ? { ...injectorCacheStats }
    : undefined

What was reviewed but skipped

  • MIME helper duplication — the codebase already has 3 purpose-built MIME maps (engine fileServer, core helpers, my new injector helper) with intentionally different default fallbacks. Consistent with codebase pattern. Left alone.
  • extractVideoFramesRange parameter sprawl — would be an API-shape change. Out of scope for cleanup.
  • HDR/VFR preflight DRY refactor — would be a substantial extraction. Out of scope.
  • Stringly-typed format literals — refactor scope.
  • __createFrameDataUriCacheForTests naming — works fine.
  • Migrating streamingEncoder/chunkEncoder to pixelFormatHasAlpha — touches files outside this stack.

Test plan

  • Full engine suite: 373 tests still pass (no test changes — purely behavioral cleanup).
  • Typecheck clean in engine + producer.
  • Lint + format clean.

Stack

Final cleanup commit, built on #435.

Copy link
Copy Markdown
Collaborator Author

jrusso1020 commented Apr 23, 2026

@jrusso1020
Copy link
Copy Markdown
Collaborator Author

Closed: cleanups rolled into new PR-C (extraction cache) in the smaller rebuilt stack. See hyperframes-notes/perf-stack-rebuild-plan-2026-04-23.md

@jrusso1020 jrusso1020 closed this Apr 23, 2026
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.

1 participant