feat(scripts): Bun.gc relief evaluation harness (SIGUSR2 GC channel, matched arms) - #2303
feat(scripts): Bun.gc relief evaluation harness (SIGUSR2 GC channel, matched arms)#2303lidge-jun wants to merge 2 commits into
Conversation
…nnel + matched-arm cells)
Phase A of the GC-relief plan: evaluate the 260731 allocator-residual gate on
Bun 1.4 before any production hook. The retention-harness child gains a
SIGUSR2 handler that runs Bun.gc(true) in the measured process and reports a
timestamped {type:"gc",durationMs} receipt (inert for the locked 7h protocol,
which never sends SIGUSR2). bun-gc-relief-eval.ts orchestrates matched
control/gc arms with SEPARATE rss-retention and latency cell types so the RSS
criterion is never contaminated by probe traffic; latency cells use a
post-intervention probe stream as the p99 oracle.
Note: Bun.spawn handle.kill() does not deliver SIGUSR2 on Bun 1.4 —
process.kill(pid) is used deliberately.
Smoke-verified end to end (OCX_GC_EVAL_SMOKE=1): GC receipts flow, report.json
written. Real 3-run evidence lands with the macmini-cf measurement pass.
Unit: devlog/_plan/260822_260822-bun14-followup-memory/020
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
✅ Deterministic PR hygiene checks passed. |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed exact head 28e8232edaab14c64fc080527f255e4fe063fd63 against the stacked base and the controlling 260731/020/040 measurement contracts.
The production boundary is good: this PR does not add a runtime GC hook. I am requesting changes because the harness cannot yet produce the evidence it says it produces.
-
runCellrecordsrssAfterLoad,rssPlus5s, andrssPlus60s, but no pre-load RSS. The controlling gate is “at least 50% of post-load RSS growth is gone,” which requires a baseline and a normalized recovery fraction.rssAfterLoad - rssPlus60salone cannot distinguish recovery from ordinary baseline drift. The plan also says the 1.4 evaluation readsextraMemorySizeand reports the counter changes, but this harness never calls/api/system/memory; the #2302 dependency is therefore unused. Capture baseline/after-load/+5s/+60s memory snapshots, including the optional extra-memory counter, and calculate the criterion from the baseline-relative growth. -
The measurement protocol requires identical concurrent request streams for the tail-latency control.
oneTurnis awaited in a plainforloop for both load and probes, so this is a serial single-request workload. It cannot establish the documented concurrent p99 criterion. Use a bounded, identical concurrency schedule in both arms and report its parameters. Also counterbalance arm order (or otherwise remove the fixed control-then-GC ordering) so host drift is not confounded with the intervention.
Please add focused harness tests for the report math and scheduling contract, then rerun the smoke probe and exact-head CI. The parent #2302 blockers still need to be resolved independently before this stacked PR can land.
…s, Phase B not implemented Real 3-run matched-arm evaluation on two darwin/arm64 hosts (local + macmini-cf, both Bun 1.4.0): recoveredByPlus60s in the GC arm is indistinguishable from control idle drift (<0.1% of the ~300MB post-load RSS; less than control on 4/6 runs). Criterion (a) of the 260731 allocator-residual gate fails decisively; GC pause 3.9-4.6ms recorded. Production Bun.gc(true) relief stays banned; the Phase B design is retained as documentation for a future runtime that changes page-return behavior. Evidence JSONs committed under evidence/.
|
Phase A real-run verdict is in: FAIL — the 260731 gate stands on Bun 1.4. 3 fresh-process runs per arm on two darwin/arm64 hosts (local + macmini-cf), both Bun 1.4.0. Consequence: no production |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 3ee558a23877f43ebd6e7532c57af61e353b283c. The added two-host runs are useful directional evidence and support keeping Phase B disabled, but they do not resolve the measurement-contract blockers on the harness. I am keeping CHANGES_REQUESTED.
-
The committed RSS records still begin at
rssAfterLoad; there is no pre-load baseline and no baseline-relative recovery fraction. The new verdict divides the recovered bytes by the entire post-load RSS (“<0.1% of load-height RSS”), which is not the controlling criterion: at least 50% of post-load RSS growth must be gone. The data may well lead to the same FAIL conclusion, but this report cannot prove that gate withoutrssBeforeLoad,postLoadGrowth, andrecoveryFraction. -
The latency cells are still serial:
for (...) await oneTurn(...). The document now calls the p99 values informational, which is appropriate after criterion (a) fails, but the PR also states that the harness is the reusable deliverable for a future Bun runtime. In its current form it still cannot run the documented identical concurrent tail-latency control, and fixed control→GC arm order remains confounded with host drift. Either implement the bounded concurrent/counterbalanced schedule and focused report-math tests, or narrow the PR/documentation claim so this artifact is explicitly only the current serial exploratory run and is not presented as the future acceptance harness. -
The verdict heading is dated
2026-08-22, but the evidence and current review date are2026-08-21. Correct the future date. The committed reports should also carry enough provenance to reproduce/compare them (at minimum the harness/source commit and host identity label); the current JSON has neither a schema/version nor a host field.
The safe production decision remains clear: do not land a Bun.gc(true) runtime hook from these results. Exact-head CI is otherwise largely green, with macOS still running, but CI cannot repair the missing measurement fields and scheduling contract. The stacked parent #2302 remains independently blocked.
리뷰 · 우선순위 46 / 80지금 핵심. 차일드 이미 실측이 들어옴. 3ee558a. 로컬+macmini-cf, Bun 1.4.0 darwin/arm64, smoke:false. criterion (a) FAIL. load 후 RSS ~300–350MB. recoveredByPlus60s가 컨트롤 아이들 드리프트랑 같음. GC 암이 컨트롤보다 덜 회수한 런이 6중 4. pause 3.9–4.6ms. Phase B 안 넣음. 260731 게이트 유지. 그 결론은 맞음. 프로덕션 워치독에 gcRelief 넣지 말 것. 이 PR 범위가 그거임. 구멍. (1) 차일드가
해결방안: #2301/#2302 먼저. 그 다음 이 패치. Phase B/ 이 댓글은 grok-bot이 작성했습니다 |
2 participants
Summary
Verification
Checklist