docs(1961): record the enableCompileCache() CLI startup measurement — not shipped - #1966
docs(1961): record the enableCompileCache() CLI startup measurement — not shipped#1966thymikee wants to merge 1 commit into
Conversation
|
Not ready at
The docs-only scope is reasonable after these corrections. |
aef6212 to
0bb16c7
Compare
|
Thanks — all four findings addressed. Rebased onto 1. Wrong metric / omitted cold evidence / substituted route → outcome (a): redone with elapsed samplesYou were right that this was measuring the wrong quantity, and it mattered. The compile cache trades CPU for filesystem wait, and The host had also quieted (load 7–10 vs 27–46 before), so interleaved elapsed sampling is now clean. Design: both arms in one loop, arm order flipped each iteration, median + bootstrap 95% CI on the median difference. On the requested routes (warm, n=120/arm):
Cold evidence added, and it changed the conclusion's shape. Cold compile cache is a regression: Stated limitation: One correction worth flagging: my first cold harness wiped the cache only before the compile-cache arm, leaving asymmetric filesystem state and producing a bogus +52.8 ms for 2. Node version and stability — confirmed wrong, fixedVerified against the Node docs: added in v22.8.0 (not 22.1), and Stability 1.1 – Active Development on the v22.x line matching the repo's 3. Contradictory
|
|
The no-ship conclusion is supported, but the power claim at lines 156–159 (and repeated in the summary) is not. |
0bb16c7 to
10f1b92
Compare
|
You're right, and the correction makes the argument stronger. Fixed and pushed (rebased onto The error: The replacement is the direct empirical statement, as you asked: the bootstrap 95% CI on the median difference, [-26.0, +4.7] ms, lies entirely above the -50 ms ship threshold, so a >=50 ms improvement is excluded rather than merely unproven — resting on the variability actually observed in the run instead of an assumed model. No corrected power calculation substituted. Swept the whole doc, not just 156–159. Three further spots restated the same overstatement in different words:
The pooled-SD figure, the ~102 sample-size number, and every "power"/"adequately powered"/"detect" phrasing are gone; a grep for them across the doc now returns nothing. The remaining statistical claims are all direct CI-exclusion statements. The conclusion is unaffected — if anything it's better supported, since the CI-based exclusion is empirical rather than model-dependent. No-ship still rests on: every measured CI excluding the threshold, the cold-cache regression (+13 to +23 ms), and Stability 1.1 at the repo's Node floor. CI is green. |
|
The no-ship conclusion is supported, but this should not close #1961 as written: the issue explicitly requested cold and warm OS-file-cache measurements, while the retained record says every sample used a warm page cache. Change |
… not shipped
Measured module.enableCompileCache() on bin/agent-device.mjs against the issue's
>=50ms warm-run bar, and did not ship it. Harness and raw per-sample data are
retained so every interval can be recomputed:
node scripts/perf/compile-cache-ab.mjs stats \
--in docs/cli-compile-cache-startup-samples.json
Metric is elapsed wall-clock. Design: both arms in one interleaved loop, arm
order flipped each iteration (this removes order bias; it does not prove
contention was absorbed, so load is recorded per route), reported as median plus
a bootstrap 95% CI on the median difference with a fixed seed.
Warm (n=120/arm): --version -0.1ms [-0.5,+0.5]; --help -3.6ms [-4.3,-2.9];
open CLI-side path -5.1ms [-5.6,-4.5]. End-to-end `ad open` against a dedicated
throwaway simulator (n=150/arm): -6.4ms [-10.0,-3.8], which agrees with the
CLI-side figure as it should if only CLI startup is affected.
So the warm effect is small but real, and roughly ten times short of the bar. An
earlier revision reported these routes as indistinguishable from zero; that was
an artifact of a busier host, and is corrected here.
Cold cache is a regression: --help +11.3ms [+10.3,+12.4], open CLI-side path
+16.4ms [+15.9,+18.0] — the cost of writing 664-848kB of cache entries.
Records that enableCompileCache() landed in Node v22.8.0 and is Stability 1.1
Active Development at the repo's >=22.12 floor.
Only the warm OS-page-cache leg was measured; `sudo purge` is unavailable
non-interactively here, so the cold-page-cache leg the issue also asked for is
untested and no claim is made about which way it would move. That leg is why
this is Part of, not Closes.
Per the issue's own "measure first, ship only if it's a real win" instruction,
the code change is reverted and only the measurement is retained.
Part of #1961
10f1b92 to
423960f
Compare
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. Top changed packed filesNo changed packed files. |
|
All four addressed. Rebased onto 1. Scope →
|
|
The revised scope and cold-page-cache limitation are now accurate, the retained samples reproduce all published medians/CIs, and the no-ship conclusion is robust. Three small correctness fixes remain before readiness:
All 28 checks are green and the corrected methodology need not change the decision. |
Summary
Issue #1961 asked to measure
module.enableCompileCache()on the CLI bin entry before shipping it — "measure first, ship only if it's a real win," bar = warm-run delta >= 50ms. Measured and not shipped. The code change was reverted; this PR retains the writeup, the harness, and the raw samples.Part of #1961, notCloses— the issue asked for cold and warm OS-file-cache measurements and only the warm leg could be run here (see Limitation below).Reproducibility
Harness at
scripts/perf/compile-cache-ab.mjs, raw per-sample elapsed times atdocs/cli-compile-cache-startup-samples.json. Every median and interval below recomputes without re-running anything:The bootstrap uses a fixed seed, so a recompute reproduces the published intervals exactly.
Methodology
Elapsed wall-clock. Both arms are copies of the bin entry against the same unchanged
dist/, sampled in one interleaved loop with arm order flipped each iteration, reported as median plus a bootstrap 95% CI on the median difference.Interleaving removes order bias; it does not prove contention was absorbed. This host is shared and load was observable but not controllable, so load is reported per route rather than as one global claim.
Numbers
Warm compile cache — the issue's bar (n=120/arm)
--version--helpopenCLI-side pathA small but statistically real gain on the heavier routes — roughly ten times short of the bar.
ad openend to end, real iOS simulator (n=150/arm)ad open SettingsAlso a coherence check: -6.4 ms end to end matches the -5.1 ms CLI-side gain, which is what should happen if only CLI startup is affected and the remaining ~1.2 s of device work is untouched.
Run on a dedicated throwaway simulator (every booted sim was claimed by another session; the device-claim guard correctly refused them). A first attempt was discarded, not reported — it was launched while the new sim was still doing first-boot work and Spotlight was indexing it, driving load to 120 on 12 cores.
Cold compile cache — a regression (n=60/arm, both arms wiped identically)
--helpopenCLI-side pathThe cost of writing 664–848 kB of cache entries. Misses are routine: keyed per Node version, in
os.tmpdir(), subject to tmp reaping.Decision: no-ship
Limitation: cold OS page cache unmeasured
sudo purgerequires a password unavailable non-interactively here, so only the warm-page-cache leg was measured. No claim is made about which way a cold page cache would move the comparison — a previous revision asserted it would penalize the compile-cache arm; that was speculation and has been removed. Settling it needs the same interleaved A/B on a host where page-cache eviction can run between samples, which would change the baseline arm's cost too.Tests
pnpm lint,pnpm typecheck,pnpm build,pnpm formatclean.bin/agent-device.mjsis byte-identical tomain.Part of #1961