perf: bound compile cache reads to one opened stream

[Iviesever]

Final status: merged, with an explicit performance tradeoff

Merged as 92defbb49b3752dc9dbc11f95fc4a4df3bcdb9a7 after the owner's delegated final review. The former Draft/package/performance hold is closed. Exact reviewed head is 2c87b47d9fe4945a75077a8ded0bdfed5ca425ab; original base is 8d9008a6031755feccf15d72a52c9fad137b4ad9. VERSION stays 5.4.0. No tag or Release was published by this operation.

What was accepted

Four files: standard-library bounded-reader helper, compile-cache transport integration, existing compile-cache tests and docs/BOUNDED_COMPILE_CACHE_READER.md. Successful loads size/read from one opened stream, keeping the original 64 MiB bound, complete-read/EOF checks, cache v2/v3/v4 decoder and failure fallback. Failed-open pathname queries retain missing/error classification. No dependency freshness, signatures, conservative retry, target scheduling, other cache loaders or CLI output is changed.

Native C++ #672, complete Native Release/self-host/package #575 and the original correctness/evidence workflows passed. The initial platform-specific directory assertion failure was fixed by deriving the strict historical category on the active standard library, not by loosening the test. No unresolved review threads were found.

Final fixed external decision experiment

Candidate Merge Decisions run 34146614041, artifact 10028122485, ZIP SHA-256 7466f9f8c2e11ba8b18b6795232460b849c09ad171fdf62f02a69216ae0afcec.

The #157 comparison uses exact original main/head Release binaries, not #158/#159 combined. Forty alternating timings-OFF pairs per warm case on one Windows comparison runner, identical fixture/cache pathnames, plus six reset cold pairs. A practical warm-regression flag was fixed before execution: median > max(0.5 ms,3%) and bootstrap median interval entirely positive. None triggered it; that is not proof of zero regression.

Scenario	Paired median ms	Paired median %	Faster pairs
small default	-0.0981	-1.10%	24/40
common129 default auto	-1.1122	-2.72%	35/40
common129 default j1	-2.7542	-5.00%	39/40
common129 verbose	-1.4428	-3.46%	34/40
static129 default	-1.3783	-3.35%	35/40
static129 verbose	-1.7587	-4.28%	37/40
PCH default	-0.1468	-1.42%	25/40
true discovery persistent hit	-0.0407	-0.42%	21/40
common129 cold	+55.3739	+1.20%	1/6

Auto's bootstrap paired-median interval is [-1.48875,-0.99975] ms; j1's is [-3.04015,-2.36495] ms. Small no-op's interval crosses zero. These are empirical evidence, not production-tail guarantees.

The cold cost is accepted, not discarded or explained entirely by overlapping cache-read work. Warm compile-cache work improves consistently across prior and new evidence, and the narrow same-stream transport has integrity value. This justifies merging without advertising cold or universal acceleration. A failed open before missing diagnostics is a known miss-path cost.

All warm cases preserve project cache/artifact metadata and audited non-output counters, with no tools/cache writes. Full metadata inventories were checked for equality but not serialized by this decision harness; raw transcripts, identities and successful assertions are retained. The three-candidate archive contains 978 pairs and 4200 verified raw transcript hashes; only the #157 experiment supports this PR's incremental decision.

Earlier evidence is not replaced

• Performance #495, initial implementation a3cdc8f, digest fa77b9ad2831c6baacb329c56984d8be9689bdc417a57a1422f8d9fc80b693df.
• Performance #496, reviewed final head, digest ec13ce6e0ef01fc1af59f820d779af81a1453f4e809f15b07275e960dca1aa7e.

Both have all 19 original scenarios and four pairs, including adverse samples. Final-head ordinary timings-OFF no-op was +1.143 ms / +10.22%, while common129 compile-cache read work fell by 4.163 ms and cold compile-cache read work increased by 9.437 ms. Work durations overlap and include decoding; they are not elapsed I/O shares. The follow-up does not erase those observations, pool the runs or add their percentages. Skipped metadata runs are not measurements.

Related decisions

#158 was closed without merge for insufficient incremental benefit; it is not implicitly approved here. #159 has its own product/performance decision and normal up-to-date protected-branch checks. Archive's missing total read/write bound is a separate iteration. Final release preparation remains separate and must compare the released v5.4.0 baseline cumulatively, not sum PR percentages.

[Iviesever]

Review addendum: remaining route from the exact main baseline

The code diff is still one commit / four files, with VERSION unchanged. Additional read-only review produced these prioritization details; they are deliberately not mixed into the compile-reader implementation:

1. Finish this transport experiment before expanding it. #492 is the latest successful retained Performance Evidence run for PR perf: separate target compile inspection from miss-only execution #156's final head; #493 and #494 were skipped, not newer measurements. Net benefit of this PR remains unproven until its own Performance Evidence #495 completes. Cache payload counters should not fall simply because pathname preflights disappear.
2. Remaining cache readers are not identical. Link and discovery have pathname type/existence + size preflights. Archive already obtains a stream length, but currently has no total-byte limit and reads quoted strings directly; its follow-on bounded-reader work needs consistent read/write bounds, corruption coverage and static-library ABBA fixtures. Toolchain's roughly 1.4 ms read-work scope includes age, PATH, newest-toolset, binary-stamp and environment-trust validation; none may be removed as if they were redundant I/O.
3. Object handoff is not a free deletion of a probe. IncrementalCompileInspection currently discards output snapshots. The target barrier revalidates shared dependencies after miss execution, while the downstream link inspection separately observes objects. A future private, invocation-owned handoff must handle all-hit, mixed hit/miss, missing/modified objects between stages, failed execution and conservative full retry. If a new barrier merely repeats the same object probes, report zero net I/O saving rather than a smaller link-only counter. Keep library, side-output and independently invoked link/archive freshness checks.
4. Reprofile the serial residual before prioritizing CLI cosmetics. Historical 129-TU artifact-layout wall time is about 9 ms; ProjectArtifactLayout::for_source still canonicalizes source identity. Investigate genuine cross-phase identity duplication before string micro-optimization, without removing junction/alias handling. Reporting's redirected-output wall time is around 0.05 ms, so a default summary is currently a UX proposal, not a demonstrated major speedup. Measure terminal/redirected/verbose behavior separately and retain all warnings/errors.
5. Specialized stores remain conditional. Discovery hits return before saving; the historical discovery-no-op write therefore warrants classifying identity misses versus filesystem-evidence invalidation, not assuming unconditional persistence or multi-key thrashing. Link-resolution work is roughly 0.25–0.26 ms in these scale fixtures. Require representative alternating-key/library-heavy evidence before either specialized reuse project.
6. Final cumulative baseline is pinned separately. Released v5.4.0 resolves to d041668de836b9eb9a36e2d6b96ff2114c5c358a, not this PR's main base 8d9008a... even though both report VERSION 5.4.0. Final cumulative performance must compare the released tag to the frozen final candidate with compatible timing definitions, retain all raw paired observations, and avoid summing percentages from successive PRs. Only the separate release-preparation PR changes VERSION/changelog.

Cache pack still requires credibly isolated remaining I/O >=25–30% of the relevant elapsed path; overlapping work totals are not that measurement. Daemon/watcher/USN/resident processes remain v6.0.

Validation snapshot: Documentation, Build Plan, Final Closure Cross-Stage, Incremental Inspection, PCH Inspection and all three Module inspection workflows have passed. Native C++, Native Release and Performance Evidence #495 are still in progress. The current Debug product build and its include-search / __has_include / transitive-library freshness and linker-side-output repair checks have passed, but that is not the complete native suite. Keep this PR draft; no universal or candidate-specific speedup is claimed yet.

[Iviesever]

First independent ABBA result and native-test correction

Performance Evidence #495 completed successfully for exact base 8d9008a6031755feccf15d72a52c9fad137b4ad9 versus initial head a3cdc8fe9a6e25fc67aa3538d9ab2f92ccb9bbb2.

Run: https://github.com/Iviesever/msvc-quick-build/actions/runs/34128610217
Artifact: 10021470066, SHA-256 fa77b9ad2831c6baacb329c56984d8be9689bdc417a57a1422f8d9fc80b693df.

All 19 scenarios and all four alternating pairs are retained. Negative deltas mean faster. Percentages are medians of paired percentages, not a ratio of the separate medians. With four pairs, the workflow's nearest-rank P95 equals the largest observed delta and is not a robust operational tail estimate.

Scenario	Base median ms	Candidate median ms	Paired median delta	Paired median %	MAD ms	P95 delta ms
129-TU no-op	41.719	35.721	-5.999 ms	-13.11%	4.767	-1.122
129-TU auto no-op	42.559	34.865	-4.171 ms	-10.48%	3.568	+0.417
129-TU j1 no-op	50.752	45.818	-2.351 ms	-4.92%	2.870	+27.014
129-TU common-header no-op	38.830	37.741	-0.561 ms	-1.46%	1.034	+1.266
129-TU single-TU rebuild	162.967	148.889	-24.490 ms	-13.32%	34.854	+23.978
ordinary no-op	4.592	5.261	+0.284 ms	+6.11%	0.526	+2.208
ordinary cold	1703.760	1785.012	+56.508 ms	+3.30%	40.639	+121.322
scale cold	4247.751	4692.642	-75.531 ms	-1.76%	973.540	+5500.084

The 129-TU target-scale-no-op improves in all four pairs: -16.010, -10.655, -1.343, -1.122 ms. Its compile-cache read work falls in all four pairs, with a paired median reduction of 12.661 ms. This does not hold for every no-op variant: j1 has an adverse cache-read pair. Work durations overlap and include decoding, so they are not elapsed-time percentages.

The instrumented top-level counters remain equal. Common-header no-op retains 129 compile hits, one link hit, 131 payload opens, zero cache writes, 424 physical snapshots, 1408 shared-evidence reuses and zero tool launches.

Retained tradeoffs, not universal acceleration

Small-target total times are mixed. The ordinary no-op paired median regresses by 0.284 ms. There is a real cache-miss transport cost: opening first can add a failed open before missing-path diagnostics; scale-cold compile-cache read work increases by a paired median of 8.429 ms. This does not explain the entire +5.500 s adverse cold pair, which remains unlocalized. No cold-path or universal speedup is claimed, and adverse samples are not discarded.

Native failure and focused correction

The initial Windows Debug and Release native runs failed shard 0/4; the inspected Debug log reports exactly one assertion failure in the new directory cache-path case. The test assumed every file_size(directory) fails. Microsoft's STL implementation can instead obtain directory size through GetFileAttributesExW; the historical loader then fails opening the stream, giving file_open_failed, not file_read_failed. The production fallback already preserves that behavior.

Commit 2c87b47d9fe4945a75077a8ded0bdfed5ca425ab changes only the test: derive the strict expected category from the same historical pathname-size preflight. It still rejects directories, never accepts a cache miss, and does not loosen the assertion to accept arbitrary failures. All production code, helpers, serializers, freshness checks, benchmark harness and VERSION are unchanged.

The new exact-head CI and ABBA are separate evidence. The first-head native failures remain in the record; initial self-host/package gates were skipped after the native failure and are NOT claimed as passed. This PR remains draft pending final-head native validation and assessment of the retained performance tradeoffs.

[Iviesever]

Measurement-boundary clarification for the remaining roadmap

Further source review refines the reporting conclusion in the earlier roadmap note. CountingStreambuf in cpp/src/app/diagnostics/PerformanceTimings.cpp measures calls to sputc, sputn and pubsync; it does not enclose the full result-reporting loop. In particular, display_source, UTF-8 path-label construction and formatting in Application.cpp happen outside that reporting sub-timer. Session destruction does flush observed streams before its timing snapshot, so the specific issue is not an omitted final flush: it is the narrower measured scope plus redirected-output workload.

Therefore the historical roughly 0.05 ms reporting field means observed stream-forwarding/flush work, not the entire cost of generating the report. It does not justify declaring default no-op summaries pointless. Keep reporting as a separate, low-correctness-risk candidate and compare the entire report block / end-to-end default and verbose invocations in terminal and redirected modes before placing it below the more invasive object-handoff work. The compile-reader choice remains supported by its own ABBA results; no output behavior is changed in this PR.

The cumulative release baseline also has a concrete schema difference: released v5.4.0 (d041668...) emits timing schema 1, without attribution/counters; current main emits schema 2. The current benchmark harness already preserves absent attribution/counters as null, which is appropriate. Final cumulative evidence must not fabricate zero-valued historical counters or retrofit instrumentation into the released baseline and still call it the original release. Use comparable shared total/phase fields with explicit timing-boundary caveats, and include external end-to-end runs with timing disabled for the large warm-path fixtures as well as the existing small no-op case. Never mix external-stopwatch totals with internal mqb.timings totals in a single percentage.

[Iviesever]

Follow-up verification and explicit merge hold

Rechecked exact head 2c87b47d9fe4945a75077a8ded0bdfed5ca425ab: Native Release #575 (run 34130603126) is now fully successful, including self-host and package job 101773028933 completed at 2026-09-07T14:22:20Z. This closes the package-validation item that was pending in the PR description. Native C++ #672 and Performance Evidence #496 are successful; subsequent Performance #497 was skipped, not a new measurement.

The owner explicitly requested no immediate merge. This PR remains Draft and unmerged. Passing package validation does not resolve or erase the already documented mixed timing-disabled/cold-path performance evidence.

The next bounded-reader iteration is isolated in #158, stacked on this exact head: only link/discovery transport, their integration tests and the design note change. Its ABBA baseline is this PR's head rather than main, so any incremental result does not re-attribute the compile-reader change. No VERSION, tag or Release change.

[Iviesever]

Merge decision after renewed owner authorization

The owner now delegates the merge decision to this review (superseding the earlier mandatory hold). I recommend not merging yet. Exact head remains 2c87b47d9fe4945a75077a8ded0bdfed5ca425ab; native Debug/Release and self-host/package gates are successful. There is no newer valid ABBA measurement beyond #496 (#497 was skipped).

The remaining gate is evidentiary, not a discovered correctness failure: warm compile-reader work improves, but the timing-disabled end-to-end no-op still has mixed paired results and the failed-open cold-path cost is real. #158's incremental comparison cannot settle this main-to-#157 question. Do not treat a green package check as new performance evidence.

Keep this PR open/Draft, with the adverse data intact. Next reporting work will branch directly from main 8d9008a6031755feccf15d72a52c9fad137b4ad9, not stack on this experiment; it has no bounded-reader dependency. This allows independent progress and attribution without either premature merge or an expanding dependency chain. No VERSION/tag/release change.

[Iviesever]

Final merge decision: accept with an explicit cold-path tradeoff

The owner's delegated review is now closed: merge exact head 2c87b47d9fe4945a75077a8ded0bdfed5ca425ab. Native Debug #672, complete Release/self-host/package #575 and original ABBA #495/#496 remain part of the record. No unresolved review threads were found.

A single fixed follow-up experiment compared the exact original main/head binaries, not a combination with other candidates: Candidate Merge Decisions run 34146614041, artifact 10028122485, ZIP SHA-256 7466f9f8c2e11ba8b18b6795232460b849c09ad171fdf62f02a69216ae0afcec. The same Windows comparison runner measured 40 alternating pairs per warm case with timings OFF, plus six reset cold pairs. Raw data and all adverse samples are retained at https://github.com/Iviesever/msvc-quick-build/actions/runs/34146614041/artifacts/10028122485 . Source, tree and binary identities are recorded. The practical warm-regression flag was declared before measurement (median > max(0.5 ms,3%) and bootstrap median interval above zero), not tuned afterward; absence of that flag is not proof of zero regression.

Paired medians: common129 auto -1.1122 ms / -2.72% (35/40 faster), j1 -2.7542 ms / -5.00% (39/40), verbose -3.46% (34/40), static129 default -3.35% (35/40). Auto's bootstrap median interval is [-1.48875,-0.99975] ms; j1's is [-3.04015,-2.36495] ms. Small no-op is -0.0981 ms / -1.10%, with an interval crossing zero, not a claim of small-target acceleration. True persistent-discovery-hit and PCH cases pass no-write/no-tool postconditions.

Cold is +55.3739 ms / +1.20%, only 1/6 faster. This adverse measured result is accepted as a tradeoff, not discarded or attributed entirely to the previously observed +9.437 ms overlapping cache-read work. Cold rebuilding is not claimed faster. The repeatable larger warm benefit and same-stream bounded-read integrity justify the narrow change; uncertainty about every noisy end-to-end sample is no longer an indefinite release blocker.

All warm cases preserve project artifact/cache metadata and match audited non-output counters. No product or benchmark code was modified between the compared binaries. VERSION stays 5.4.0; no release/tag operation. #158 is decided separately and is not implicitly approved.