Repository navigation
docs(build): finish the off-cycle compile audit, removing nothing - #7602
Conversation
Quest-Claim: 88a5a29c289aaed6
The MP4 reader candidate changes library IR by only ten lines, grows test IR by 904, and leaves rebuild CPU and instructions flat. Preserve the measured patches and keep production readers unchanged. Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Thin reduces the final FFI unit from 135.70 to 88.04 seconds, but the whole command uses 547.74 rather than 390.19 CPU seconds for the same 474 units. The archive grows from 62.4 to 71.4 MiB, and the paired session benchmark establishes no runtime win. Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
kixelated
changed the title
…uest A one-line moq-net edit through mbx rebuilds the same 26 units as plain Cargo in 99 G instructions against 450 G once incremental state exists; the first edit costs 538 G while recording it. Final binaries are about half of a warm edit. Nothing is removed, so the off-cycle quest finishes and its checkpoint becomes the audit's results. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 84ae577.
No actionable bugs found. Keeping the existing readers and fat LTO is a reasonable, simpler outcome: the retained data supports rejecting the reader consolidation, separates final-unit timing from whole-build cost, and distinguishes first-edit incremental setup from warm edits. The recurring audit remains intact after removing the one-off quest.
Verification: static review of the documentation and recorded measurements; checked the headline timing totals and recomputed the 72-case runtime geometric mean from the JSON. I did not reproduce the builds, run tests, or execute benchmarks, so this does not independently validate the recorded measurements.
(Written by OpenAI)
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Docs/quest-only PR that finishes the off-cycle compile-time audit; no production code, dependencies, tests, profile, API, or wire change.
- New measurement record (
.github/compile-time-audit.json): raw environment, llvm-lines tables, dev/release--timingsunit data, reader-candidate patches, shipping-LTO comparison, and the mbx incremental probe. - Recurring audit refreshed (
quest/m2/audit-compile-time.md): rewritten "what to check first" bullets plus an "Off-cycle run (2026-11-01)" section; the old baselines section is retitled "Historical baselines". - Quest finished:
quest/m1/prune-build-time.mddeleted and its index bullet removed fromquest/m1/README.md.
I spot-checked the prose against the JSON: the six-target IR table, cold dev/release wall/CPU/unit durations, the fat-vs-thin LTO table (390.19→547.74 s CPU, 62.4→71.4 MiB), the 72-case session-bench geomean (1.034, range 0.747–1.403), and the mbx incremental table all match. Base SHA 21d6da9fc1 resolves, cargo build-ffi exists, the embedded reader-candidate patch passes git apply --check, #6176 is merged and #7594 is open as described, and no dangling prune-build-time references remain. No action required.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
#6985 merged after the off-cycle run's base, so point feature-unification work at the still-open hakari quest and flag which baselines it can shift. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The first comparison built fat into a target the default release build had already filled, so 300 of its 474 units were timed at 0 s. From fresh targets thin cuts the whole build's wall by 12.5% at about equal CPU. Fat stays for now, with the reason restated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 4631447.
No new actionable bugs found. The cold-target rerun corrects the earlier unequal-work LTO comparison: the recorded 474 unit identities match, their durations sum to the reported totals, and the revised 12.5% wall saving, 1.7% CPU increase and 14.5% archive growth are consistent. Keeping the superseded result explicitly labeled, separating pre-hakari baselines, and leaving the shipping-profile decision unchanged is a reasonable, low-complexity outcome; the original whole-build argument for fat LTO no longer applies, as the revised guidance acknowledges.
Verification: static review of the changes since the prior review, excluding merged base changes, plus arithmetic checks against the retained JSON. I did not reproduce builds, run tests or benchmarks, or independently validate measurement conditions.
(Written by OpenAI)
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Incremental re-review of the 3 commits since the last pullfrog review (84ae577c); all docs and measurement data, no production surface.
- Cold-target LTO rerun (
46314474f): rebuilt fat and thin from fresh per-profile targets, replacing the earlier comparison that had filled fat into the default-feature release target.quest/m2/audit-compile-time.mdand.github/compile-time-audit.jsonnow record fat 465.48 s / thin 407.12 s wall (-12.5%), finalmoq-ffiunit 162.63 s / 114.87 s (-29.4%), +1.7% CPU, archive +14.5% (both under the 100 MB ceiling). I recomputed each percentage against the JSON and they hold. - Superseded run preserved under
shipping_lto.superseded, with the reason (300 of 474 fat units timed at 0 s; thin rebuilt them at 202.16 s) matching the prose. - Workspace-hack note (
fa1f1bec2): the m1 baselines now flag that they predate#6985and point feature-unification work at/quest/m0/ci/hakari.md. - Baseline bullet rewritten to state fat LTO is kept on the maintainer's 2026-10-20 choice (archive condition met, thin cheaper on wall) rather than on a whole-build cost claim.
The one maintainer-facing open question (fat vs thin LTO) is a future decision the PR body explicitly flags, not a defect in this diff. Both prior reviews on 84ae577c and head 46314474ff found nothing actionable.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
Problem
quest/m1/prune-build-time.mdis a one-off, off-cycle run of the recurring compile-time audit. It defaults to removing whatever dominates dev, release, and incremental build time, and records before and after numbers. This PR finishes it.Approach
quest/m2/audit-compile-time.md) with six-target IR baselines, cold dev/release builds, owned script and macro costs, the rejected reader candidates, the shipping-profile LTO comparison, the 72-case paired session benchmark, and a real mbx incremental edit. Raw data and exact candidate patches are in compile-time-audit.json.rs/moq-net/src/lib.rsthree times, then revert it. Each edit rebuilds the same 26 dependent units. Through mbx, once incremental state exists, an edit costs 99.0 G instructions, 24 to 26 s CPU, and 12 to 14 s wall. Plain Cargo (MBX_DISABLE=1,incremental = false) costs 450 G every time (-78% for mbx). The first edit through mbx costs 538 G (+20%) while it records that state. In a warm edit the five final binaries are 49 to 53% of unit time. moq-net itself takes about 2.3 s.default-memberswould only shorten a bare rootcargo build, because CI andjustselect packages or pass--workspace. No public feature removal is proposed.Coordination
21d6da9fc1), so the recorded baselines predate the workspace-hack. The audit now says so, notes which rows it can shift (only builds with dev-dependencies, such as the mux test-profile IR count), and points feature-unification work at the still-open hakari quest instead of the merged PR.Impact
Alternatives
default-members: rejected, since it only affects a bare root build.Choices made without asking, for review: keeping fat LTO and the current readers, and finishing the quest with no removal. Note that the 2026-10-20 LTO decision said "keep fat LTO until the archive shrinks": the archive has shrunk (m0's fat Go-mirror archive was 71.2 MiB; fat
libmoq_ffi.ais 62.4 MiB here) and thin now wins on cold wall time, so switching to thin is a fair question for the maintainer. This PR keeps fat because the runtime cost stays unresolved and release builds stay off the PR gate.Checks
Run on head
46314474ff(after mergingorigin/main):quest gate nix develop --command just ci origin/main: exit 0. Selected modules: check markdown, check gh, test gh, check quest, check shell, large_files, toml, nix, justfile. This docs-only diff selects no Rust, JS, or flake module.quest check --base origin/mainandgit diff --check: pass.cargo build-ffi --release --timingsat base21d6da9fc1, fat then thin, each into its own freshCARGO_TARGET_DIR, withMBX_DISABLE=1,CARGO_BUILD_JOBS=2,CARGO_INCREMENTAL=0, underquest gate. Both exited 0 with identical 474-unit sets.Review
quest review --base origin/main, session01a19d5c-22f6-7d03-bdc1-3911cab3964b).46314474ff7bdc88ca03a8407f02258b30ce45f8(round 2).fa1f1bec22, after the main merge): one finding, fixed. "Equalize cache state before comparing whole-build LTO costs": the fat run had 300 of 474 units timed at 0 s. Both profiles were rerun from fresh targets and the prose, table, and JSON replaced; the old run's summary is kept undershipping_lto.superseded.84ae577c87(OpenAI review, Pullfrog) reported nothing actionable; CodeRabbit was rate-limited.Follow-ups
moq,moq-studio,moq-bench,moq-relay). A future audit could measure whether the linker or the binaries' own codegen dominates. Not planned here.(Written by Claude Opus 5.5)