Repository navigation
fix(cli): installing Chrome on a busy Windows PC no longer takes ten minutes - #7301
Merged
Merged
Conversation
Contributor
Edit accuracy: accurate 267 (base branch 267), smooth 104 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (2)
|
Contributor
There was a problem hiding this comment.
Reviewed at d260c3be. Requesting changes. The one blocker is in the PR body, not the code.
Blocker
- The body has no
fails without the fix:line. Lane rule: a fix PR must name the test that fails without the fix on a line that starts withfails without the fix:. Here the test is named only in prose under the Test plan ("…fails without the fix, and it is the only one that does"). The marker in the test file's comment doesn't count. Add a line such as:
fails without the fix: renderSetupWorker.priority.test.ts > "the Chrome install runs one step above normal priority"
Changing only the body doesn't change the head. I'll approve this same head once the line is there.
What I verified (the code is ready to approve)
- The claim holds. With core built locally,
renderSetupWorker.priority.test.tspasses 7/7 at head. WithrenderSetupWorker.tsfrom main, exactly one test fails: "the Chrome install runs one step above normal priority". - Coverage: every Chrome download goes through the
browser-installworker (manager.ts→installInChild→runRenderSetupWorker("browser-install")). So the raised priority reachesbrowser ensureand the first-render install alike.mode === "browser"(ensureBrowser) only downloads through that same child. - Refusals: EPERM and EACCES on Unix stay silent and the install still runs, as before. Other errors are still reported, now worded per mode. The refusal test checks both.
- The lint priority behaviour is unchanged. The refactor only parameterizes
lowerPriority. - All CI workflows on this head have completed green. The one
CIrun still in progress at review time was a duplicate.
Non-blocking
- On Windows,
ABOVE_NORMAL_PRIORITY_CLASSis not inherited by child processes. Only IDLE and BELOW_NORMAL are. So the systemtarunzip, which takes 16 s under load according to your notes, still runs at normal priority. That matches your "not exercised" note. Just confirming the fix covers only the download.
Contributor
There was a problem hiding this comment.
Re-reviewed at d260c3be. Approved.
- The blocker is resolved. The body now has:
fails without the fix: "the Chrome install runs one step above normal priority" in renderSetupWorker.priority.test.ts … 1 failed, 6 passed.This matches my local run. WithrenderSetupWorker.tsfrom main, exactly that test fails, 1 failed and 6 passed. At head, all 7 pass. - The head is unchanged since my last review, so the code checks and green CI from that review still apply.
- My non-blocking note still stands: on Windows, ABOVE_NORMAL priority isn't passed to child processes, so the
tarunzip still runs at normal priority.
SanHsien
added a commit
to SanHsien/hyperframes
that referenced
this pull request
Nov 6, 2026
…view Adopts heygen-com#7299 (accented Latin letters draw in a bundled family at its bundled weight), heygen-com#7301 (installing Chrome on a busy Windows PC), heygen-com#7262 (cloud render logs a line a minute while a job holds its status), heygen-com#7265 (video starts a second early and catches up hidden) and heygen-com#7305 (check_report names the finding that stopped the check). Player packed budget takes upstream's +1655 from heygen-com#7265 since the player bundles core's media-steer. Defers heygen-com#7302; heygen-com#7261 and heygen-com#7263 do not apply. Watermark advances to 9e2510b. Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com> Co-authored-by: WaterrrForever <miao.yang@heygen.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
What
The first-use Chrome download now runs one step above normal CPU priority, so a busy Windows PC no longer turns it into a many-minute wait.
Why
On a busy Windows machine the download barely moved. Every small disk write in Node's file stream queues for CPU behind whatever else is running, so the transfer crawled at a few hundred KB/s. Data kept arriving, so the 60 s stall check never fired, and a person just saw
Still getting Chrome readyfor ten minutes or more while the process used about 2 s of CPU.Plain
curlfetched the same file in about 3 s under the same load, and a Node download that doesn't write to disk took about 4 s.How
renderSetupWorker.tsraises its own priority inbrowser-installmode only. That process lives for the download and unzip, and spends nearly all of it waiting. Most Unix systems refuse this without privileges, and the install then runs at normal priority as it does today, which is why that refusal is not an error.I chose this over changing how the file is written because the write happens inside
@puppeteer/browsers, andinstall()has no option for it.Test plan
Measured on a 4 vCPU
windows-latestrunner with 12 busy-loopnodeprocesses, 119.5 MB:@puppeteer/browsersinstall(), no unzipinstall()at this priorityrenderSetupWorker.priority.test.ts: 7 passed, 3 runs in a row on a Linux box.fails without the fix: "the Chrome install runs one step above normal priority" in
renderSetupWorker.priority.test.ts, with the priority call removed at this head: 1 failed, 6 passed.browser ensurebuilt from source, same runner and load, one run each:mainhyperframes@0.9.18 browser ensureunder that load also never finished: killed by my limit at 900 s, and again at 420 s.Not exercised yet:
tartook 16 s under the same load and runs in a child process.No visible change
No interface changes. The only difference a person sees is a shorter wait on first use.
Size
43 lines, most of them tests. It is a lone fix on purpose: no open PR touches this worker, and the change is one priority call.