Repository navigation
perf(producer): overlap the plan step's audio work and hash files once - #5777
Merged
Merged
Conversation
Contributor
Edit accuracy: accurate 271 (base branch 271), smooth 183 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
jrusso1020
approved these changes
Oct 17, 2026
jrusso1020
left a comment
Collaborator
There was a problem hiding this comment.
Approving at a3fa10e2. I checked the byte-equivalence claim by reading every path that feeds an artifact digest or the plan hash. I didn't re-run the bench.
Why the outputs can't change
- Audio inputs are the same.
runAudioStagewrites only underworkDir(MIXED_AUDIO_FILENAMEandaudio-work/), which is.plan-work, outside the three trees being hashed and uploaded beside it. It only readscompiledDir, nowfinalCompiledDir. That's the same tree after the rename (stagedcompiled/no longer exists at that point), andresolveProjectRelativeSrcstill resolves againstprojectDirfirst. So the overlap can't race a file being hashed or uploaded. - The hashes are the same function.
sha256Filemoved verbatim fromplanV2.tstoplanHash.ts, andsha256FileAsynchashes the same byte stream.computePlanHashgets the same{path, sha256}list. Only the source of each sha changed: before,readFileSyncandsha256Hex; now the memo or a streamed read. - Nothing in the prehashed trees changes after the prehash. Everything written into
planDirafter the audio block lands inmeta/,plan.jsonand the audio path. None of that is prehashed: the memo only misses on them, and freeze fills it after writing them. Themeta/videos.jsonwrite happens before the audio block. The memo is per plan (new Map()inplanV2WithPublisher), so a reusedplanDircan't hit a stale entry. - Upload integrity holds without the local re-hash.
uploadContentAddressedFileToS3is a singlePutObjectCommand(not multipart) withChecksumSHA256set to the expected digest, so S3 checks the full object and rejects a local file that drifted. The new test covers that. TheIfNoneMatch: "*"conditional create and the HEAD reuse check are unchanged. - Pre-uploaded blobs end up in the manifest. The
#publishedDigestsearly return inputBlobhappens afterassertSha256, so only already-published content-addressed blobs skip the HEAD.artifactTargetsships everycompiled/file to every chunk, so nothing uploaded in the audio window is orphaned, and the new test checks the manifest matches a serial build.
Non-blocking
Promise.allSettledmeans a fast audio failure still waits for the prepublish uploads to finish before the plan throws and the publisher aborts. That's correct, just slower on the failure path. Racing the abort is an option if it ever matters.sha256Fileins3Transport.tsmay now have no callers in the package. Worth a quick check whether anything else imports it.- As the body says, the GCS planner and the non-deferred path (prehash over extracted frame trees) are covered only by unit tests.
Required CI is green at this head. No live run.
— Rames
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On a 30-minute 1080p30 project the distributed plan step on AWS Lambda drops from about 112 s to about 88 s. The audio mix now runs alongside hashing and uploading the project files, instead of in front of them, and each file is hashed once instead of up to four times. Plan artifacts and final videos are byte-identical to main.
What changed
buildLocalExecutionPlanmovescompiled/and the video trees into their final places first. It then starts the audio stage and, in parallel, streams a hash of those trees.planV2WithPublisheradds awhileAudioMixescallback that uploads thecompiled/blobs in that same window, using the existing 32-slot pool.artifactTargetsships everycompiled/file to every chunk, so each pre-uploaded blob is in the manifest; the new test checks this.compiled/dir. Its contents are the same copy of the project, so the audio input bytes and ffmpeg args don't change.PlanFileDigestsmemo (sha256 by absolute path) is filled by that streamed prehash. It is then read byfreezePlan, by the publication's plan-hash recompute, and by the per-artifact digests.readFileSynceach whole file, including the full 902 MB video. It now streams.ChecksumSHA256, and S3 rejects bytes that don't match.S3PlanV2ArtifactPublisher.putBlobreturns early for a digest it already published, so blobs uploaded during the audio window are not checked again with a HEAD afterwards.[plan] stage {"stage","ms"}): copy, compile, probe, extract, audio, prehash, prepublish, freeze, publication, upload, manifest. The Lambda handler addsplan_stagelines for download and untar.What I measured
All runs used the same 30-min fixture: a 902 MB H.264 background, 29 MB of AAC, and a GSAP title, 16 chunks. Everything ran on one 10 GB x86 Lambda stack, swapping only the function code. Each plan was a direct invoke of the
planaction.Byte identity:
audio.m4a, exceptmeta/videos.jsonandplan.json.meta/videos.jsonembeds the planner's random staging directory, so it also differs between two runs of main. With that one path normalized it is identical.plan.jsondiffers only because it carries the resulting hashes.Tests (bun, each file 3 runs in a row):
What I did NOT exercise
gcsTransportstill hashes locally before its upload, since GCS has no SHA-256 check on its side.video-sources/and are not pre-uploaded during the audio window, so they still upload after it. This fixture's video lives in the project, so it ships undercompiled/.