perf(producer): stage frames via a junction before copying on Windows#2314
perf(producer): stage frames via a junction before copying on Windows#2314sidorovanthon wants to merge 2 commits into
Conversation
|
@sidorovanthon rebase main pls, after that I'll review again |
9d5eb34 to
abf3c6e
Compare
|
@miguel-heygen rebased onto current The original diff predated your A Windows junction needs neither Developer Mode nor Administrator, so the common no-privilege case now stages the frame dir at link speed instead of copying every frame (your own comment flags the copy as "measurably slower"). The copy stays as the last resort for mounts with no link support at all (SMB/NFS/exFAT), where even a junction fails with a capability error; a non-capability junction error (e.g. Title/description updated to match. Ready for another look. |
miguel-heygen
left a comment
There was a problem hiding this comment.
The junction implementation is currently bypassed by the exact local Windows path this PR is intended to optimize. renderOrchestrator.ts passes materializeSymlinks: shouldCopyExtractedFrames(process.platform), and shouldCopyExtractedFrames("win32") is true; stageExtractedFrameDirOnce(..., true) calls cpSync(recursive) directly and never reaches linkOrCopyFrameDir, so neither the plain-symlink nor junction tier runs. A fresh 0.7.58 Windows field report reproduced the consequence with one 305.453s video: all 9,164 JPEGs were extracted, then the process exited while materializing the cache into compiledDir. Please close the class in this PR by routing local Windows staging through the link/junction/copy ladder (while keeping distributed-plan materialization as a real copy), and add an orchestrator-level regression proving the actual win32 local-render call does not request eager copy. The helper-only tests currently pass while the production caller still selects the old path.
`linkOrCopyFrameDir` already degrades a rejected symlink to a full recursive copy of every extracted frame (EPERM/EACCES/UNKNOWN — Windows without Developer Mode). Copying is measurably slower than linking. Insert a junction attempt between the symlink and the copy: a Windows junction needs neither Developer Mode nor Administrator, so the common no-privilege case now stages the frame dir at link speed instead of copying. On POSIX the "junction" type is ignored by Node, and that tier is only reached on a privilege error POSIX never raises, so behavior there is unchanged. The copy fallback stays as the last resort for mounts with no link support at all (SMB/NFS/exFAT), where even a junction fails with a capability error; a non-capability junction error (e.g. ENOSPC) still propagates. `MaterializeFileSystem.symlinkSync` gains an optional `type?: "junction"` param; the default fs-backed impl already forwards it. Tests: junction fast-path (symlink EPERM → junction succeeds → no copy); copy only when both symlink and junction fail; non-capability junction error propagates. 145 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WiwMWMe25vjrDaiEsVFwQe
…ction/copy ladder
The junction fast-path was dead code on the exact platform it targeted.
The local render invoked the extract stage with
`materializeSymlinks: shouldCopyExtractedFrames(process.platform)`, and
`shouldCopyExtractedFrames("win32")` is `true`, so `stageExtractedFrameDirOnce`
called `cpSync(recursive)` directly and never reached `linkOrCopyFrameDir` —
neither the plain-symlink nor the junction tier ran. A 0.7.58 Windows field
report reproduced the consequence: 9,164 JPEGs extracted, then the process
exited while materializing the cache into `compiledDir`.
Fix the class, not the symptom:
- `renderOrchestrator.ts`: the local in-process render passes
`materializeSymlinks: false`, routing win32 through the
symlink -> junction -> copy ladder. A junction needs neither Developer Mode
nor admin, so the common no-privilege case stages at link speed; the
recursive copy stays as the ladder's last resort (SMB/NFS/exFAT).
- Delete `shouldCopyExtractedFrames`: the platform->copy coupling is exactly
what bypassed the ladder, and the junction tier now owns the win32 privilege
case. Removing it makes the bypass structurally impossible to reintroduce.
- Tighten the `materializeSymlinks` JSDoc: eager copy is reserved for
distributed `plan()` (self-contained planDir); do NOT recouple it to
`process.platform`. `plan.ts` is unchanged.
Regression proven at the stage the orchestrator actually calls
(`runExtractVideosStage`), not a helper: the local path does not request an
eager copy (flag `false` or omitted), and only the distributed path does.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LNERhhFUhVkVBn3SksgeZq
abf3c6e to
33e0789
Compare
|
@miguel-heygen thanks - you were exactly right, the junction ladder was dead code on the very platform it targeted. Rebased onto current Root cause. The local render invoked the extract stage with Fix:
Behaviorally a strict improvement: the ladder's tier 3 is the same Verification (Windows): producer unit lane 306 passed; Ready for another look. |
What
Rebased onto current
mainand re-scoped. WhenlinkOrCopyFrameDircan't create a plain symlink for an extracted-frames dir, it now tries a junction before falling back to the full recursive copy.Why
mainalready degrades a rejected symlink (EPERM/EACCES/UNKNOWN— Windows without Developer Mode/Administrator) tocpSync(recursive). That copy is correct but, as the code comment itself notes, measurably slower — every frame of every video is physically copied into the compiled dir.A Windows junction is a directory link that needs neither Developer Mode nor Administrator. So the common no-privilege case can stage the frame dir at link speed instead of copying it. The copy stays exactly where it was, but now only as the last resort for mounts that support no links at all (SMB/NFS/exFAT).
How
linkOrCopyFrameDirbecomes three tiers:EPERM/EACCES/UNKNOWN. On POSIX Node ignores the"junction"type, and we only reach this tier on a privilege error POSIX never raises, so POSIX behavior is unchanged. A junction needs a local absolute target and doesn't exist on SMB/NFS/exFAT, so a capability error here (EPERM/EACCES/UNKNOWN/EINVAL/ENOSYS/EOPNOTSUPP/ENOTSUP, seeisSymlinkCapabilityError) drops to tier 3. A non-capability junction error (e.g.ENOSPC) still propagates.MaterializeFileSystem.symlinkSyncgains an optionaltype?: "junction"parameter; the defaultfs-backed implementation already forwards it, and the only other staging path (materializeSymlinks, distributedplan()) is untouched.Test plan
renderOrchestrator.test.ts—materializeExtractedFramesForCompiledDirsuite:junction fast-path: plain symlink throws
EPERM, junction succeeds →cpSyncis not called, frames remapped exactly as in the symlink happy path.copy only when both fail: plain symlink and junction both throw
EPERM→ plain then junction attempted (asserted via the recordedtypeargument), thencpSync(recursive).UNKNOWN variant unchanged (both link attempts fail → copy).
non-capability junction error (
ENOSPCfrom the junction attempt) → rethrown, no copy.Unit tests added/updated (
renderOrchestrator.test.ts: 145 passed / 0 failed via vitest)bunx oxlint/bunx oxfmtclean;tsc --noEmitclean; lefthook pre-commit (lint/format/fallow/typecheck) greenDocumentation updated (if applicable) — behavior/perf-only change, no docs affected
🤖 Generated with Claude Code