Repository navigation
test(budget): give a Windows child spawn the cold-start headroom it measures - #4830
Conversation
…easures The first proxy child in native-profile-startup.test.ts published its port at 50.7s against a 45s SPAWN_BUDGET_MS while the next spawn in the same file was ready in 1.8s. Gate the budget to 90s on win32 only, the same way BULK_DURABLE_IO_BUDGET_MS already is.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthrough
ChangesSpawn budget configuration
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No concrete merge-blocking risk is evident; Windows startup tests receive additional time while other platforms are unchanged. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
리뷰 · 우선순위 74 / 80이 PR은 제품 로직이 아니라 테스트 예산(숫자) 만 고친다. 지금 고치는 파일은 하나다. 왜 지금 부작용으로 라인 34 - 경로 경로 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
|
Maintainer integration into Exact head |
…sable port (#4834) PR #4830 raised SPAWN_BUDGET_MS on win32 from 45s to 90s to fix one test case. 31 test files read that constant and nine hand it to setDefaultTimeout, so the edit reached 339 Windows cases: 315 went 45s to 90s, 22 more that multiply it went 90s to 180s, and one derivation chain in codex-sync-api reached 265s. The detectors that now report twice as late are the contention ones -- codex-write-lock, the cross-process history-lock exclusions, the shim process cases -- and several of the affected files never spawn anything. The measurement behind #4830 was also contaminated. The child published its port through atomicWriteFile, the production SECRET writer, which on Windows runs hardenSecretPath(..., required: true) twice, each able to spawn PowerShell for SID resolution and several 30s-budgeted icacls passes. That ACL ceremony ran inside the window the parent measures as "time to reach a port" -- on a disposable port number. - SPAWN_BUDGET_MS returns to 45s on every platform. - COLD_SPAWN_BUDGET_MS (90s, win32 only) is consumed exactly once, by the readiness wait of the first proxy child in native-profile-startup. - The child publishes its port and settled markers with a plain temp-file rename, preserving the #1061 no-partial-read contract without the ACL ceremony. - The child logs child-entry, start-server-begin, start-server-end and port-published, so the next slow run names its own phase. - codex-sync-api owns its bounds instead of deriving them, which returns that case to 130s and makes it immune to the next edit of a shared constant. It also reports its measured preparation window on green runs. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
…easures (lidge-jun#4830) The first proxy child in native-profile-startup.test.ts published its port at 50.7s against a 45s SPAWN_BUDGET_MS while the next spawn in the same file was ready in 1.8s. Gate the budget to 90s on win32 only, the same way BULK_DURABLE_IO_BUDGET_MS already is. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
…sable port (lidge-jun#4834) PR lidge-jun#4830 raised SPAWN_BUDGET_MS on win32 from 45s to 90s to fix one test case. 31 test files read that constant and nine hand it to setDefaultTimeout, so the edit reached 339 Windows cases: 315 went 45s to 90s, 22 more that multiply it went 90s to 180s, and one derivation chain in codex-sync-api reached 265s. The detectors that now report twice as late are the contention ones -- codex-write-lock, the cross-process history-lock exclusions, the shim process cases -- and several of the affected files never spawn anything. The measurement behind lidge-jun#4830 was also contaminated. The child published its port through atomicWriteFile, the production SECRET writer, which on Windows runs hardenSecretPath(..., required: true) twice, each able to spawn PowerShell for SID resolution and several 30s-budgeted icacls passes. That ACL ceremony ran inside the window the parent measures as "time to reach a port" -- on a disposable port number. - SPAWN_BUDGET_MS returns to 45s on every platform. - COLD_SPAWN_BUDGET_MS (90s, win32 only) is consumed exactly once, by the readiness wait of the first proxy child in native-profile-startup. - The child publishes its port and settled markers with a plain temp-file rename, preserving the lidge-jun#1061 no-partial-read contract without the ACL ceremony. - The child logs child-entry, start-server-begin, start-server-end and port-published, so the next slow run names its own phase. - codex-sync-api owns its bounds instead of deriving them, which returns that case to 130s and makes it immune to the next edit of a shared constant. It also reports its measured preparation window on green runs. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
… they now measure (#4843) The first round added the instrumentation; this one uses it. Run 35141541461 measured the Windows preparation window at 2740ms and the Linux/macOS one at 423-575ms, against a reserve that had been guessing at 52.7s. Boot drops 40s to 30s and the Windows preparation reserve 80s to 55s, which takes the case from 265s to 95s while still leaving room for the 52.7s outlier the reserve was written for: that much preparation still leaves the flip its full boot budget and its reap. The same run also showed every readiness wait in native-profile-startup at 2.0-4.9s on all six Windows shards, first child included, confirming the 50.7s observation behind #4830 was the ACL ceremony rather than a cold start. COLD_SPAWN_BUDGET_MS is kept as a bound against the part one run cannot rule out, with a note to delete rather than raise it. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
1 participant
The failure
devhas been red for five consecutive Cross-platform CI runs. One of the two remaining Windowsfailure classes is this:
Run
35118018849, job104895935554,windows 1/6. The interesting line is three lines later inthe same log:
[native-startup] port-published elapsedMs=50728. The child was not hung. Itpublished its port 5.7 seconds after the harness gave up, and the very next case in the same file
was ready in 1759ms.
Why raise the budget rather than the test
tests/helpers/test-budget.tssets two conditions for raising a budget. Both hold here. The waitis intrinsic: the spawned proxy child reaching its port file is the assertion, which is why the
file already carries two comments recording 8-19s and 10-18s spawns on loaded Windows shards.
And the ablation still fails: nothing about this change makes a child that never publishes pass —
it fails at 90s instead of 45s.
What 45s was measuring on Windows is runner contention. The leg runs four Bun pools on one runner
and the first spawn in a file pays a cold start the rest do not.
The change
SPAWN_BUDGET_MSbecomes 90s onwin32and stays 45s everywhere else, following the shapeBULK_DURABLE_IO_BUDGET_MSalready uses in this file for the same reason. Every other lane keepsthe shorter signal.
Validation
No local suite, typecheck or build was run. The evidence is the hosted run above: the measured
publish time, the passing sibling spawn, and the file's own recorded Windows spawn costs.
Summary by CodeRabbit