diff --git a/.agents/docs/2026-08-13-module-impl-split-report.md b/.agents/docs/2026-08-13-module-impl-split-report.md new file mode 100644 index 00000000..e90eccfe --- /dev/null +++ b/.agents/docs/2026-08-13-module-impl-split-report.md @@ -0,0 +1,483 @@ +# Module interface / implementation separation — report + +**Branch:** `refactor/module-impl-separation` (PR #545) vs `main` @ `b1563fe` +**Date:** 2026-08-13 +**Machine:** Linux 6.8, 32 cores, gcc@16.1.0 via mcpp 2026.8.11.3 + +## 1. What changed + +Every `.cppm` in `src/` carried both the module interface and its own +implementation. Each is now a standard-conforming pair: + +| | file | module declaration | produces a BMI? | +|---|---|---|---| +| interface unit | `X.cppm` | `export module M;` | yes | +| implementation unit | `X.cpp` | `module M;` | **no** | + +No directories moved. A partition's implementation goes into an implementation +unit of the *primary* module (`module M;`), because `module M:part;` would be +another partition and would still produce a BMI — and a module may have any +number of implementation units. + +**What did change beyond moving text**, stated plainly rather than filed under +"no behaviour changed": + +- **11 call sites in product code** now name their stream — + `std::println(stdout, ...)` instead of `std::println(...)`. `[print.fun]` + defines the short form *as* the long one, so the meaning is identical, but it + is an edit to product code and not a move (§8b explains why it was necessary). +- **The musl release target moved from `gcc@15.1.0-musl` to `gcc@16.1.0-musl`** + (§8a). A different compiler major generates different code; this changes what + ships, even though no source changed to cause it. +- **Namespace-scope `static` lost the keyword** on the entities that split, taking + them from internal to module linkage. Same single definition either way, and + module linkage still keeps the name module-private. +- **Moved bodies lost their implicit `inline`**, which costs cross-TU inlining + without LTO. Measured: +0.10% binary size at `-O0`; `-O2` runtime is not + measured. + +Behaviour is otherwise unchanged, and it is checked rather than asserted: 1,262 +function definitions conserved exactly, 961 exported identifiers with none lost +or gained, 110 module names byte-identical, and the full unit suite plus the e2e +block green on six CI workflows. + +Four things left the interface: + +1. **Namespace-scope implementations** — 81 files, 779 entities, 26,606 body lines. +2. **Class member bodies** — 29 files, 286 members, 3,845 body lines. A body + defined inside a class is implicitly `inline`, so it stayed in the BMI. +3. **Module-private declarations** — a non-exported helper's declaration stays in + the interface only if something still *there* names it (an exported template, + an `inline`/`constexpr` body, a class member body). Otherwise it belongs in + the implementation unit: in the interface it sits in the BMI, so changing its + signature would recompile every importer for nothing. +4. **The imports the bodies took with them** — 153 of 554 interface import edges + (28%). `split.py` *copies* the import list into the implementation unit, which + is right for the implementation and wrong for the interface: it was left + importing modules only the moved bodies used, and every such edge makes the + interface's BMI depend on a module it does not name. Found by self-review, not + by a compiler — nothing about it fails to build. + +### Interface surface + +| | lines | share of original | +|---|---|---| +| interface before | 46,253 | 100% | +| interface after phases 1–3 | **14,666** | **32%** | +| implementation after | 35,655 | | + +110 interface units, 92 implementation units (90 generated + `main.cpp` + +`windows_syslibs.cpp`). Only one implementation unit is trivial, and it is the +pre-existing `windows_syslibs.cpp` — none of the 90 generated units is an empty +translation unit added for nothing. + +The interface is what every importer reads through the BMI. That is the number +the build speed follows. + +## 2. Why it should matter — the fan-out that was there before + +Measured over the 99 local modules: + +| module | direct importers | transitive downstream | +|---|---|---| +| `xlings.platform` | 44 | **66** | +| `xlings.core.log` | 40 | 54 | +| `xlings.core.utils` | 8 | 44 | +| `xlings.core.config` | 35 | 41 | +| `xlings.core.palette` | 8 | 53 | +| `xlings.libs.json` | 31 | 52 | + +Average transitive downstream: **10.7 modules**. Before the split, editing one +line of `platform.cppm`'s implementation invalidated its BMI and recompiled 66 +translation units. After it, that edit lands in `platform.cpp`, which produces +no BMI. + +## 3. Method + +Both refs are measured **in one worktree**, checking each out in turn: same +path, same filesystem, same toolchain fingerprint, same warm dependency cache. +The only variable is the source content. + +Only `src/` is swapped — `mcpp.toml` is held at the branch's version for both +sides. That is deliberate: the dev target is `gcc@16.1.0` either way, so holding +the manifest constant keeps the comparison to the source content and keeps the +release-toolchain bump (§8a) out of it. + +- **Cold build** — `rm -rf target && mcpp build`, repeated. The project's own + target directory is removed so every project TU recompiles; the global + dependency cache (`~/.mcpp/build-cache`) stays warm on purpose, because the + dependencies are not what changed. +- **Incremental build** — one implementation edit, timed rebuild. The edit + inserts a statement into a function *body*: a plain `touch` would not do, + because mcpp preserves a BMI's timestamp when a recompile produces + byte-identical content, so touching a `.cppm` skips every downstream unit and + flatters the before-picture. Same function, same statement, on both refs — the + file it lands in is the only difference, which is exactly what is being + measured. +- **`mcpp test`** — the whole unit suite, 38 binaries. Each test file is a + non-module TU that imports many modules, so this is where interface size shows + up most directly. It is also the gate a developer actually waits on. + +Probe functions span the fan-out range **and** the two kinds of body, because +they are not equivalent. A non-`inline` free function's body need not be in the +BMI at all, so main may already avoid downstream work for it; a body defined +inside a class **is** implicitly inline and therefore certainly in the BMI. If +the split only helped the second kind, the first kind's numbers would say so. + +| probe | module | transitive downstream | on `main` it is | +|---|---|---|---| +| `parse_sudo_env` | `xlings.platform` | 66 | namespace-scope fn | +| `level_string` | `xlings.core.log` | 54 | namespace-scope fn | +| `strip_ansi` | `xlings.core.utils` | 44 | namespace-scope fn | +| `parse_index_repos_json` | `xlings.core.config` | 41 | namespace-scope fn | +| `shim_filename_` | `xlings.core.xself.doctor` | 16 | namespace-scope fn | +| `workspace_install_targets` | `xlings.core.config` | 41 | **class member** (in the BMI) | +| `compare_segment` | `xlings.core.semver` | 23 | **control** — `inline`, unmoved | + +`compare_segment` is the control: it stays `inline` in `semver.cppm` on **both** +refs, so an edit to it invalidates semver's BMI either way and the **same** +downstream set rebuilds. It therefore separates two effects that would otherwise +be confounded: + +- **rebuild set** — an implementation edit stops invalidating the BMI at all, so + nothing downstream rebuilds. Only the moved probes get this. +- **rebuild cost** — whatever does rebuild downstream is now a small interface + instead of a large one. The control gets this too. + +The control is expected to improve, just much less. If it improved as much as the +moved probes, the split would not be what the numbers are measuring. + +Harness: `.agents/tools/module-split/{cold.sh,incr.py}`, driver +`.agents/tools/module-split/measure-side.sh`. + +## 4. Measurements + +### Cold build — every project TU recompiled + +| ref | runs (s) | median | +|---|---|---| +| `main` @ b1563fe | 67.17 / 55.27 / 56.40 | **56.40** | +| branch, phases 1–2 | 36.97 / 27.28 / 26.85 | 27.28 | +| **branch, phases 1–3** | 32.96 / 25.71 / 26.09 | **26.09** | + +**2.16× faster**, and the direction was not a given: translation units roughly +doubled (110 interfaces → 110 interfaces + 92 implementation units). Smaller +BMIs pay more than the extra TUs cost. The first run on each side is the high +one — that is page cache for freshly written files, which is why the median is +the figure quoted. + +### Binary size + +| ref | dev binary (`-O0 -g`) | +|---|---| +| `main` | 124,238,424 B | +| this branch | 124,368,072 B (**+0.10%**) | + +Dropping implicit `inline` costs cross-TU inlining without LTO, so this was +worth checking rather than assuming. At `-O0` the answer is "no change worth +naming"; a release-mode (`-O2`) size and runtime comparison is **not** measured +here. + +### One implementation edit + +Two runs per probe. Both are shown rather than a median: with n=2 a median *is* +the mean, so one outlier would move it silently. + +| probe | downstream | `main` (s) | branch (s) | best-to-best | +|---|---|---|---|---| +| `parse_sudo_env` | 66 | 63.12 / 54.04 | 4.46 / 4.60 | **12.1×** | +| `level_string` | 54 | 62.42 / 51.38 | 4.49 / 4.56 | **11.4×** | +| `strip_ansi` | 44 | 62.63 / 50.01 | 4.64 / 4.71 | **10.8×** | +| `workspace_install_targets` (class member) | 41 | 49.00 / 58.95 | 6.43 / 6.31 | **7.8×** | +| `parse_index_repos_json` | 41 | 62.13 / 49.08 | 19.69 / 6.36 | **7.7×** | +| `shim_filename_` | 16 | 45.55 / 35.89 | 7.86 / 7.78 | **4.6×** | +| `compare_segment` — **control**, `inline` on both | 23 | 43.58 / 54.53 | 17.24 / 17.54 | 2.5× | + +Those branch figures are from phases 1–2. Spot-checked again after phase 3, both +improved slightly and neither regressed: `parse_sudo_env` 4.39 / 4.35 (from +4.46 / 4.60) and the control 15.31 / 15.24 (from 17.24 / 17.54) — the control gains +because the downstream interfaces it still rebuilds now carry smaller import +closures too. Best-to-best for `parse_sudo_env` becomes **12.4×**. + +**On `main`, editing one function body costs about as much as building the whole +project from scratch** — 54–63s against a 56.40s cold build. That is the shape +the fan-out predicted: a high-fan-out interface edit rebuilds nearly everything, +and before the split every body edit *was* an interface edit. + +The control lands at 2.5× while the moved probes land at 4.6–12.1×. The control's +gain is the interfaces downstream simply being smaller to recompile; the gap +between the two is what moving the body out actually bought. + +Both columns are phases 1–2. On the final tree the control is 2.9× +(43.58 → 15.24) and `parse_sudo_env` is 12.4× (54.04 → 4.35) — the ratio between +them, which is the part that matters, is unchanged. + +The branch's `parse_index_repos_json` first run (19.69s against 6.36s on the +second) is an outlier, not a pattern — the same probe measured 6.51 / 6.32 in an +earlier run of the same harness. + +### `mcpp test` — and the finding that changed the conclusion + +| ref | wall clock | vs main | result | +|---|---|---|---| +| `main` | 953.81s | — | 38 passed, 0 failed | +| branch, phases 1–2 | 1104.83s | **1.16× SLOWER** | 38 passed, 0 failed | +| **branch, phases 1–3** | **921.73s** | **1.03× faster** | 38 passed, 0 failed | + +Before phase 3 this was the one measurement that went the wrong way, and the +report said so. Per test binary: + +| ref | min | median | max | sum over 38 | vs main | +|---|---|---|---|---|---| +| `main` | 20.86 | 21.18 | 41.16 | 832.5 | — | +| branch, phases 1–2 | 23.96 | 24.92 | 45.53 | 1011.7 | **+3.74s** | +| branch, phases 1–3 | 20.88 | **21.07** | 41.30 | **831.0** | **−0.11s** | + +**I had the mechanism wrong, and the fix is what proved it.** The regression looked +like link cost: the overhead was near-constant per binary regardless of the test +file's own size, and each test binary now links ~202 object files instead of ~112. +That reasoning does not distinguish the two candidates — **BMI loading is also +near-constant per TU**, because it depends on the import closure and not on the +file being compiled. + +Phase 3 settled it as a controlled experiment. It changes **only** interface +import edges: the object count each test binary links is identical before and +after. If linking were the cause the penalty would have survived. It vanished +entirely — +3.74s per binary became −0.11s. + +So the cause was the stale imports: every test TU was loading the BMIs of modules +that nothing in the interfaces named, 28% more import edges than needed. The +split does **not** meaningfully trade link time for compile time; it was carrying +a defect that looked like that trade. + +The phases 1–2 figure also carries a ~27s pessimism — `src/` was regenerated +part-way through that run — but that is noise next to the 179s the trim removed. + +## 5. Two gcc@16.1.0 internal compiler errors + +Both are compiler crashes, not code errors, and both blame the wrong entity. +Both are recorded with their evidence in `outline.py`'s `ICE_SKIP`, and the two +members keep their bodies inline. + +**`Counts::issues()`** (`core/xself/doctor.cppm`) — a two-line accessor. +Defining it out-of-line segfaults cc1plus at `DoctorState st;` in +`doctor.cpp:55` — a different type, in a different function. + +**`Config::instance_()`** (`core/config.cppm`) — +`static Config& instance_() { static Config inst; return inst; }`. Its +function-local static is where the module-attached `Config` is first completed. +Move that body and the first-instantiation point moves with it, after which +cc1plus segfaults compiling an **unrelated translation unit** — `doctor.cpp`, on +a different type, in a different module. Nothing in the message names anything +that changed. Found by bisecting config.cppm's 85 movable members +(`bisect-member.sh`): #34 is the boundary and the other 84 are fine. + +A crashed cc1plus leaves a truncated `.gcm`, after which unrelated targets fail +with `Bad file data` on the next build. `target/*/gcm.cache` and `~/.mcpp/bmi` +both need clearing after one, or the next run reports a failure that has nothing +to do with what you changed. + +## 6. Deliberately not done + +**Vestigial `inline` is left alone. 1,953 lines still sit in `inline` bodies in +the interface:** + +| lines | file | +|---|---| +| 315 | `core/semver.cppm` | +| 315 | `core/elf_same_source.cppm` | +| 259 | `ui/layout.cppm` | +| 217 | `core/closure_check.cppm` | +| 180 | `core/subos.cppm` | +| 71 | `core/palette.cppm` (53 transitive downstream) | + +In a module interface, `inline` means "put this body in the BMI so importers can +inline it". Removing it is a performance-visibility decision, not a move — the +maintainer's call, not a mechanical migration's. Stripping it would take the +interface down by roughly another 13% and is the largest remaining lever. + +Measured on `main`'s own tree, the figure is **1,953 lines** — identical. So these +are not lines the split left behind or failed to move; they are exactly what was +already `inline` before it started, carried over untouched. + +## 7. Invariants checked statically + +Three things this refactor must not change, checked against the pre-split tree +rather than assumed: + +| invariant | before | after | tool | +|---|---|---|---| +| exported identifiers | 961 | **961** (0 lost, 0 gained) | `export-surface.py` | +| module names | 110 | **110, byte-identical** | — | +| comment lines | 10,316 | 10,612 | — | + +The export-surface check had to be made depth-aware to mean anything: a +line-anchored regex over an `export namespace` body also matches the **local +variables** inside the function bodies that live there, so the first version +reported 914 "lost exports" with names like `1`, `a`, `acc`, `activeBin` — an +artefact of the bodies moving, not a lost export. + +The comment surplus is the comment above an `#if`, emitted to both units along +with the directive. Getting there took a fix: dropping a module-private +declaration from the interface first deleted the comment above it, 230 lines +explaining helpers whose bodies are still there. Counting comment lines is what +found that; the compiler had nothing to say about it. + +## 8. Three toolchains, three failures the dev build could not see + +The dev loop is one compiler (`gcc@16.1.0`, x86_64-linux-gnu). This project ships +three more, and each one rejected something the dev build accepted. + +### 8a. `gcc@15.1.0-musl` — a range pipeline in an implementation unit + +`x86_64-linux-musl` (then `gcc@15.1.0-musl`) rejected `catalog.cpp`: + +``` +use of 'constexpr auto std::ranges::views::__adaptor::operator|(...)' +before deduction of 'auto' +``` + +The body is byte-identical to main's. The same `std::views::transform` pipeline +compiles in an interface unit and not in an implementation unit — under that +compiler. `mcpp build --target x86_64-linux-musl` reproduces it locally: one +error, one file. + +**Fixed by moving the musl target to `gcc@16.1.0-musl`** rather than by keeping +the function in the interface. The reverse has bitten this project before: +`views::split | ranges::to` compiled under 15.1.0-musl and made a whole module +fail with "Bad file data" under 16.1.0, blaming an unmodified TU +(`.agents/docs/2026-08-06-subos-architecture-proposal.md` §590). Range adaptors +in modules are fragile in **both** directions across those two versions, so the +two targets now share one compiler major instead of trading one breakage for the +other. + +A stale object file hid this on the dev toolchain too: several regeneration +rounds cleared only `target/*/gcm.cache` and kept the `.o` files, so +`catalog.o` was never recompiled. Before believing a green build: a cold +`rm -rf target`. + +### 8b. `llvm@20.1.7` — the stream-less `std::print` in an interface template + +macOS failed on the phase-2 push having passed on phase 1. Every TU +instantiating a zero-argument `log::` template died inside libc++'s ``: + +``` +call to deleted constructor of + 'formatter>, char>' +``` + +That is `std::print` no longer picking its `FILE*` overload and deducing `stdout` +**as** the format string. libc++ implements `print(fmt, args...)` as +`print(stdout, fmt, args...)`, so the `FILE*` overload has to win overload +resolution at the point of instantiation — and for a template that stays in the +interface, that point is in the **importer**. Once enough bodies leave the +interface, clang 20 stops picking it. + +Two rules fix it, and neither needs a per-entity exclusion: + +- **11 call sites across 4 interfaces name their stream** — `core/log.cppm` (8) + and one each in `platform/{linux,macos,windows}.cppm`. `[print.fun]` defines the + two-argument form *as* `print(stdout, ...)`, so behaviour is identical and the + deduction is gone. Bodies that move to an implementation unit are left alone: + nothing instantiates those from another TU. (The commit message says 23 sites + in 6 files; that was the count of stream-less prints in the *pre-split* + `.cppm` set, and 12 of them belong to bodies that then moved to a `.cpp` and + were never normalised. 11 in 4 is what the tree carries.) +- **8 implementation units gained ``** — `config.cpp`, `cmdprocessor.cpp`, + `xim/downloader.cpp`, `platform.cpp` and the four platform partitions — because + a body that ends up there can name `stdout`/`stderr`/`FILE` while its interface + has no global module fragment at all. `config.cppm` has none, and + `Config::print_paths()` lands in `config.cpp`. + +**Two wrong fixes came first, and both are reverted rather than left in the +tree.** The first restored three declarations to `log.cppm` on the theory that +interface reachability drove it — macOS then failed again, identically, which +disproved it. The second pinned in-class bodies containing a stream-less print +as "instantiation anchors"; it worked, but it was treating the symptom, and once +the streams were named it was unnecessary. + +**The real lesson is the loop, not the bug.** `llvm@20.1.7` on **Linux** +reproduces the macOS failure exactly — main builds clean, the split does not — so +this was diagnosable in 40 seconds all along, and three CI cycles were spent +before trying it. A *minimal* probe of the same shape does **not** reproduce it; +it needs the whole project, which is what made the failure look +macOS-libc-specific. `.agents/tools/module-split/clang-variant.sh` builds any variant with clang +in an isolated copy; `clang-bisect.sh` binary-searches one file's members. + +### 8c. My own bug, for the record + +`open(f, 'w').write(ensure_cstdio(open(f).read()))` truncates the file before the +read runs, so every implementation unit taking that path came out **empty**. It +surfaced as undefined symbols at link time — never as an error in the file that +was emptied. + +## 9. The gate this work should have started with + +Four compilers, and each one caught something the others accepted. All four are +now runnable locally, and the first three take under a minute each: + +```bash +rm -rf target && mcpp build # gcc@16.1.0 (dev) +mcpp build --target x86_64-linux-musl # gcc@16.1.0-musl (release) +bash .agents/tools/module-split/clang-variant.sh check --all # llvm@20.1.7 (macOS family) +python3 .agents/tools/module-split/export-surface.py +``` + +The cold `rm -rf target` matters as much as the extra compilers: a kept object +file hid §8a on the dev toolchain. + +Windows (`llvm@20.1.7`, MSVC-flavoured) and real macOS remain CI-only. + +## 10. What this report does not verify + +- **728 lines of the moved code are never compiled by a Linux build** — + `platform/windows.cpp` (326), `platform/macos.cpp` (151), `platform.cpp` (65), + `xself/uninstall.cpp` (50) and a long tail. The macOS and Windows CI jobs are + the gate for those, and both pass. + + An earlier draft of this report said 1,228. That counter flagged any block whose + condition merely *mentioned* `_WIN32` or `__APPLE__`, which wrongly included + `#if !defined(_WIN32)` — true on Linux, and 208 of the miscounted lines were + `platform/unix.cpp`'s POSIX code that Linux does compile. The figure above comes + from evaluating the conditions with `__linux__` defined and the others not. +- Runtime behaviour beyond the unit suite: the e2e block needs a release tarball + and network access, and runs in CI (green). +- Release-mode (`-O2`) runtime performance. Dropping implicit `inline` costs + cross-TU inlining without LTO; binary size is reported, a runtime comparison is + not. + +## 11. An mcpp observation + +`mcpp` warns `module ':part' imported but not provided in this build` for a +partition import inside an implementation unit. The warning is cosmetic — the +generated dyndep edges are correct (`obj/part.o: dyndep | p4.m-part.gcm +p4.m.gcm`), verified on a throwaway project — but its check pass does not expand +the bare `:part` the way its dependency scanner does. The generated +implementation units avoid the warning by relying on the primary interface's +`export import :part;` rather than importing the partition directly. + +## 12. What self-review caught that no compiler would + +Everything below built green before it was found. That is the point: a green +build is not evidence that a refactor is complete or that a report is true. + +| finding | how it surfaced | +|---|---| +| **153 of 554 interface import edges (28%) named nothing in the interface** — split.py copied the import list instead of moving it, so the interface kept depending on modules only the moved bodies used | reading the diff and asking what the interface still needs | +| **`cold.sh` never ran from where it was committed** — its root was `dirname/../..`, right in `build/bench` and pointing at `.agents/` from `.agents/tools/module-split/`. Broken since this branch's first commit, and cited by this report | running the documented command | +| **This report cited two scripts that are not in the repo** — `clang_variant.sh` and `measure_side.sh` lived under `build/`, which is gitignored, so the local gate could not be run by anyone reading it | checking that every path in the report exists | +| **The unverified surface was overstated by 69%** — 1,228 lines "behind platform guards" is really 728; the counter flagged any block mentioning `_WIN32`/`__APPLE__`, including `#if !defined(_WIN32)`, true on Linux | re-deriving a number instead of trusting the first script that produced it | +| **The control-probe claim was backwards** — the report said it "must show no improvement". It improves 2.5× on the phase-1–2 tree and 2.9× on the final one, because the interfaces it still rebuilds are smaller | comparing the claim against the measurement | +| **A closure bug in the new tool** — `names \|= …` makes `names` local to the nested function | smoke-testing the tool against `main`, where it correctly finds only 4% to drop | +| **Tool files were 644 while every other tool in `.agents/tools` is 755** | listing the mode bits | + +Four things the review checked and found clean, each stated as a number rather +than an impression: + +| invariant | result | +|---|---| +| function definitions | **1,262 → 1,262**, none lost, none duplicated | +| exported identifiers | **961 → 961**, 0 lost, 0 gained | +| module names | 110, byte-identical | +| dynamic-initialisation order | one module has globals on both sides of the split, and the one that stayed is `std::atomic{false}` — constant-initialised, so there is no order to preserve | +| reconstructed `#if` guards | both verified against their originals; macOS and Windows CI confirm | diff --git a/.agents/docs/2026-08-13-release-2026.8.13.1-notes.md b/.agents/docs/2026-08-13-release-2026.8.13.1-notes.md new file mode 100644 index 00000000..01429f3b --- /dev/null +++ b/.agents/docs/2026-08-13-release-2026.8.13.1-notes.md @@ -0,0 +1,104 @@ +# 2026.8.13.1 —— 「接口和实现分开」 + +> 配套:PR #545。报告:`.agents/docs/2026-08-13-module-impl-split-report.md` +> 工具:`.agents/tools/module-split/`(`regen.sh` 可逐字节复现本次提交树) + +## 1. 一句话 + +110 个 `.cppm` 同时装着模块接口和它自己的实现,于是**任何函数体的改动都会改变 BMI, +所有 importer 全部重编**。这一版把它们分成标准的一对:`X.cppm`(`export module M;`, +产出 BMI)+ `X.cpp`(`module M;`,**不产出 BMI**)。 + +接口从 46,253 行降到 14,666 行。但这次真正值钱的不是那个 2.16×,而是**唯一一个 +测回来是"变慢"的数,以及它最后被证明不是我说的那个原因。** + +## 2. 先说三条被自己推翻的前提 + +都是我写下来之后被测量或编译器否掉的,记在这里而不是悄悄改掉。 + +### 2.1 「TU 翻倍,冷构建会变慢」→ **反了,快 2.16×** + +110 个接口变成 110 接口 + 92 实现单元,TU 数量差不多翻倍,所以我在计划里写的是 +"冷构建方向不确定,可能变慢"。实测 56.40s → 26.09s。**更小的 BMI 带来的收益超过 +更多 TU 的成本**,而这只有测了才知道。 + +### 2.2 「`mcpp test` 变慢是链接成本」→ **被 phase 3 证伪** + +单测套件一开始**慢 1.16×**(953.81s → 1104.83s),是唯一朝错方向走的数。 +我的论证是:每个测试二进制的开销近乎恒定(+3.74s)、与测试文件自身大小无关, +而每个二进制现在要链接约 202 个目标文件而不是 112 个 —— 所以是链接。 + +**这个论证站不住。** BMI 加载**同样**是"每 TU 近乎恒定",因为它取决于导入闭包 +而不是被编译的那个文件。我的证据无法区分两个候选原因。 + +phase 3(裁剪残留 import)成了对照实验:它**只**改接口的 import 边,每个二进制 +链接的目标文件数前后完全一样。如果病因是链接,惩罚会留下来。它消失了 —— +**每二进制 +3.74s 变成 −0.11s**,套件反过来比 main 快 1.03×。 + +真因是:每个测试 TU 都在加载接口里根本没提到的模块的 BMI。 + +### 2.3 「对照探针必须不改善」→ **改善 2.9×,而这才是它有用的地方** + +`compare_segment` 在两个分支上都是 `semver.cppm` 里的 `inline`,所以我写"它必须 +不变"。它变快了。原因是:编辑接口时两侧重编的**模块集合**相同,但分支上那些下游 +**接口本身小得多**。 + +正确的表述是它分离了两件事 —— **重编集合**(实现改动不再让 BMI 失效,只有搬移过的 +探针拿到这个)和**重编成本**(仍要重编的东西现在更小,对照组也拿到)。对照组 +2.9×,搬移过的探针 4.6–12.4×,**差距才是搬移的贡献**。 + +## 3. 数字 + +| | main | 本版 | | +|---|---|---|---| +| 接口行数 | 46,253 | **14,666** | 32% | +| 冷构建(3 次中位) | 56.40s | **26.09s** | **2.16×** | +| 改一个实现(66 下游) | 54.04s | **4.35s** | **12.4×** | +| 改一个类成员(41 下游) | 49.00s | **6.31s** | 7.8× | +| 对照(两侧都 `inline`) | 43.58s | 15.24s | 2.9× | +| `mcpp test`(38 个二进制) | 953.81s | **921.73s** | 1.03× | +| dev 二进制 `-O0` | 124,238,424 B | +0.10% | | + +同一 worktree 内切换,只换 `src/`,路径/文件系统/工具链/依赖缓存全同。 + +**main 上改一个函数体的代价约等于一次完整冷构建** —— 因为分离之前,每次改函数体 +其实都是在改接口。 + +## 4. 四套工具链,四个 dev 构建看不见的失败 + +`mcpp build`(`gcc@16.1.0`)是本项目四个编译器之一,另外三个各否掉了它接受的东西。 + +| 工具链 | 否掉了什么 | +|---|---| +| `gcc@15.1.0-musl` | 搬进实现单元的 `std::views::transform` 管道。**修法是把 musl 目标升到 `gcc@16.1.0-musl`** —— 反方向的同类问题此前咬过本项目(`views::split \| ranges::to` 在 15 上过、在 16 上让整个模块以 "Bad file data" 失败),所以两个目标现在共用一个编译器主版本,而不是用一个破口换另一个 | +| `llvm@20.1.7` | 接口 template 里**不带流参数的 `std::print`**。libc++ 把 `print(fmt,…)` 实现为 `print(stdout, fmt, …)`,所以 `FILE*` 重载必须在**实例化点**赢得重载决议 —— 而对接口 template 来说那个点在 importer 里。修法:4 个接口的 11 处显式写出流,外加 8 个实现单元补 `` | +| `gcc@16.1.0` | 两个 ICE,各自报在**另一个模块的另一个类型**上。`Config::instance_()` 是二分 config.cppm 的 85 个成员到 #34 才定位的 | +| 一个残留的 `.o` | 把第一个问题也藏过了 dev 工具链:清 `target/*/gcm.cache` 会留下 `.o`,该失败的 TU 根本没重编。**只有 `rm -rf target` 才算证明构建过** | + +`llvm@20.1.7` 在 **Linux** 上就能逐字复现 macOS 的失败,所以那本是一个 40 秒的回路, +而我在此之前用掉了三轮 CI 去猜。**最小复现不行** —— 它需要整个项目,这正是那个 +clang 可见的 bug 一开始看起来像 macOS libc 特有问题的原因。 + +## 5. 不变量:验过,不是断言 + +| 不变量 | 结果 | +|---|---| +| 函数定义 | **1,262 → 1,262**,零丢失零重复 | +| 导出标识符 | **961 → 961**,0 丢失 0 新增 | +| 模块名 | 110 个逐字节一致 | +| 动态初始化顺序 | 唯一跨单元分裂的模块里留下的是 `std::atomic{false}`(常量初始化),没有顺序可破坏 | +| 重建的 `#if` 守卫 | 两处与原文核对一致;macOS / Windows CI 实证 | + +## 6. 刻意没做 + +**残留的 `inline` 一行未动 —— 接口里仍有 1,953 行在 `inline` 函数体内。** +在 main 的树上量到的是**同一个数字**,所以它们不是本次漏搬的,而是本来就 `inline` +的原样带过来。在模块接口里 `inline` 的含义是"把函数体放进 BMI 供 importer 内联", +去掉它是性能可见性决策,该由维护者定,不该由一次机械迁移悄悄做。这也是剩下最大的 +一根杠杆(约再压 13%)。 + +## 7. 顺手记一笔(与本版无关) + +`mcpp.toml` 里 `ftxui = "6.1.9"` 在 `[dependencies]` 下,走的是**已废弃的裸名回退**, +每次构建都会 warn,而该回退**在 mcpp 2026.9 移除**。一行的事(移到 +`[dependencies.compat]`),也是 `mcpp.lock` 反复变动的原因。本轮没动,留给维护者定。 diff --git a/.agents/plans/2026-08-13-module-interface-impl-separation.md b/.agents/plans/2026-08-13-module-interface-impl-separation.md new file mode 100644 index 00000000..1bc12acc --- /dev/null +++ b/.agents/plans/2026-08-13-module-interface-impl-separation.md @@ -0,0 +1,136 @@ +# Module interface / implementation separation + +**Branch:** `refactor/module-impl-separation` +**Date:** 2026-08-13 + +## Goal + +Split every C++23 module in `src/` into a standard-conforming pair: + +- `X.cppm` — **module interface unit** (`export module M;`): types, declarations, + templates, `constexpr`. Produces the BMI. +- `X.cpp` — **module implementation unit** (`module M;`): the function bodies. + Produces **no BMI**. + +No directory moves, no behaviour changes, no new product code. + +## Why: the BMI is the recompile trigger + +Today every function body lives in the interface unit, so **every body edit +changes the BMI**, and everything that imports the module recompiles. + +Measured fan-out over the 99 local modules (`build/bench/analyze.py` + +import-graph scan): + +| module | direct importers | transitive downstream | +|---|---|---| +| `xlings.platform` | 45 | **63** | +| `xlings.core.palette` | 8 | 53 | +| `xlings.libs.json` | 31 | 52 | +| `xlings.core.log` | 39 | 47 | +| `xlings.core.config` | 34 | 35 | + +Average transitive downstream: **10.7 modules**. Editing one line in +`platform.cppm`'s implementation rebuilds 64 translation units. After the +split it rebuilds one. + +## Mechanism verified before writing any code + +`build/bench/probe2` (a throwaway 3-file mcpp project) established that mcpp +and gcc@16.1.0 support the standard shape, and that the four rules the +migration depends on actually hold: + +1. mcpp scans `.cpp` implementation units with P1689 dyndep and orders them + after their interface's BMI. `thing.cppm` → `thing.m.o` + `p2.thing.gcm`; + `thing.cpp` → `thing.o` and **no `.gcm`**. +2. A **non-exported** helper declared in the interface and defined in the + implementation unit is callable from an **exported template** that gets + instantiated in a downstream TU. It links and runs. +3. An `extern` module-linkage global declared in the interface and defined in + the implementation unit is readable from such a template. +4. The `export` keyword must be omitted in the implementation unit; a default + argument must appear only in the interface; `constexpr` + `static_assert` + stay in the interface. + +Incremental behaviour on that probe: + +| edit | recompiled | time | +|---|---|---| +| implementation body (`thing.cpp`) | `thing.cpp` only | 0.29s | +| interface (`thing.cppm`) | `.cppm` + `.cpp` + every importer | 0.79s | + +mcpp additionally preserves a BMI's mtime when a recompile produces +byte-identical content, so an interface edit that does not change the BMI +already avoids downstream work. That is why the bodies are the thing to move. + +## Classification rules + +Applied per namespace-scope (or class-scope) entity. + +**STAY in the interface — whole:** +- type definitions (`struct` / `class` / `union` / `enum`), `using`, `typedef`, + namespace aliases, `concept`, `static_assert` +- anything `template<...>` (11 sites, all variadic log/format wrappers) +- `constexpr` / `consteval` functions and variables, `inline` variables +- preprocessor conditionals that select declarations + +**SPLIT — declaration in the interface, definition in the implementation unit:** +- non-template, non-`constexpr` function definitions at namespace scope, + exported or not (a non-exported helper still needs its declaration in the + interface when a template or another staying entity calls it) +- out-of-line-able member functions of non-template classes, including + `static` member functions (defined as `T C::f(...)`, no `static` keyword) +- namespace-scope variable definitions with dynamic initialisation + +**MOVE WHOLE to the implementation unit:** +- anonymous-namespace blocks (4 files, all under `src/core/mirror/`) +- namespace-scope `static` free functions **not** referenced by a staying + entity (internal linkage cannot span two units) + +## Ordering invariant + +All namespace-scope variable *definitions* of a module move to the same unit +(the implementation unit), so their relative dynamic-initialisation order is +preserved. Never split a module's globals across the two units. + +## Scope sizing + +| | lines | share | +|---|---|---| +| outside class bodies (free functions) | 39,258 | 84.9% | +| inside class/struct bodies (need out-of-line members) | 6,995 | 15.1% | +| total across 110 `.cppm` | 46,253 | | + +71 `static ... (...) {` definitions; indent 0 = namespace-scope internal +linkage, indent 4 = static member functions (ordinary out-of-line definitions). + +Both groups are in scope. The class-heavy files are the high-fan-out ones +(`config.cppm` is 93% class body **and** has 34 direct importers), so +skipping them would forfeit much of the benefit. + +## Known trade-off, to be measured not assumed + +Moving a body out of the interface drops its implicit `inline`. Without LTO +the release build loses those cross-TU inlining opportunities. The dev build +is `-O0`; release is `-O2`. The report must carry binary size and, where +cheap, a runtime check — not a claim that this is free. + +Cold-build direction is genuinely uncertain: TU count roughly doubles +(110 → ~220), which costs, while every BMI gets smaller, which pays. Measure +both cold and incremental. + +## Verification + +1. `mcpp build` succeeds. +2. `mcpp test` — full unit suite green (must run `mcpp build` first; + `test_interface_protocol` drives the real binary). +3. e2e suite via `tests/e2e/run_all.sh`. +4. Benchmark main vs branch in this one worktree by switching branches, so + path, filesystem and toolchain fingerprint are identical and only source + content differs. + +## Baseline captured (main, this worktree) + +Cold `mcpp build` after `rm -rf target`, warm global dependency cache, 32 cores: +**68.85s / 55.14s / 64.12s**. Variance is ~25%, so the comparison needs +repeats and a median, not a single pair of numbers. diff --git a/.agents/tools/module-split/analyze.py b/.agents/tools/module-split/analyze.py new file mode 100755 index 00000000..6cc4fab7 --- /dev/null +++ b/.agents/tools/module-split/analyze.py @@ -0,0 +1,93 @@ +#!/usr/bin/env python3 +"""Size the interface/implementation split: how much of each .cppm is a +namespace-scope function body (movable) vs inside a class body (needs +out-of-line member syntax) vs must-stay (template/constexpr/type).""" +import re, glob, sys, collections + +def strip_for_scan(s): + """Blank out string/char literals and comments so brace matching is sane. + Keeps byte offsets identical.""" + out = list(s) + i, n = 0, len(s) + while i < n: + c = s[i] + if c == '/' and i + 1 < n and s[i+1] == '/': + j = s.find('\n', i) + j = n if j < 0 else j + for k in range(i, j): out[k] = ' ' + i = j + elif c == '/' and i + 1 < n and s[i+1] == '*': + j = s.find('*/', i + 2) + j = n if j < 0 else j + 2 + for k in range(i, j): + if s[k] != '\n': out[k] = ' ' + i = j + elif c in '"\'': + q = c; j = i + 1 + # raw strings R"(...)" + if q == '"' and i > 0 and s[i-1] == 'R': + m = re.match(r'"([^(]*)\(', s[i:]) + if m: + delim = m.group(1) + end = s.find(')' + delim + '"', i) + j = n if end < 0 else end + len(delim) + 2 + for k in range(i, j): + if s[k] != '\n': out[k] = ' ' + i = j; continue + while j < n: + if s[j] == '\\': j += 2; continue + if s[j] == q: j += 1; break + if s[j] == '\n': break + j += 1 + for k in range(i, j): + if s[k] != '\n': out[k] = ' ' + i = j + else: + i += 1 + return ''.join(out) + +tot = collections.Counter() +per_file = [] +for f in sorted(glob.glob('src/**/*.cppm', recursive=True)): + src = open(f).read() + scan = strip_for_scan(src) + lines = src.count('\n') + # find class/struct/union bodies at any depth: `struct X ... {` ... matching `}` + cls_lines = 0 + for m in re.finditer(r'\b(?:struct|class|union)\s+(\w+)[^;{]*\{', scan): + # skip forward declarations (handled by the `{` requirement) + start = scan.index('{', m.start()) + depth, i = 0, start + while i < len(scan): + if scan[i] == '{': depth += 1 + elif scan[i] == '}': + depth -= 1 + if depth == 0: break + i += 1 + cls_lines += src.count('\n', start, i) + tot['lines'] += lines + tot['class_body_lines'] += cls_lines + per_file.append((lines, cls_lines, f)) + +print(f"total .cppm lines : {tot['lines']}") +print(f"inside class/struct : {tot['class_body_lines']} " + f"({100*tot['class_body_lines']/tot['lines']:.1f}%)") +print(f"outside class/struct : {tot['lines']-tot['class_body_lines']} " + f"({100*(tot['lines']-tot['class_body_lines'])/tot['lines']:.1f}%)") +print() +print("files with the most class-body code:") +per_file.sort(key=lambda r: -r[1]) +for lines, cls, f in per_file[:12]: + print(f" {cls:5}/{lines:5} ({100*cls/max(lines,1):4.0f}%) {f}") + +# namespace-scope `static` free functions (internal linkage -> cannot be +# declared in one unit and defined in another) +print() +stat_fns = [] +for f in sorted(glob.glob('src/**/*.cppm', recursive=True)): + scan = strip_for_scan(open(f).read()) + for m in re.finditer(r'^([ \t]*)static\s+(?!.*\b(?:constexpr|inline)\b)([\w:<>,& *]+?)\s+(\w+)\s*\([^;]*?\)\s*(?:const\s*)?\{', scan, re.M): + stat_fns.append((f, m.group(3), len(m.group(1)))) +print(f"namespace/class-scope `static ... (...) {{` definitions: {len(stat_fns)}") +for f, name, ind in stat_fns[:15]: + print(f" indent={ind:2} {name:28} {f}") diff --git a/.agents/tools/module-split/bisect-member.sh b/.agents/tools/module-split/bisect-member.sh new file mode 100755 index 00000000..3e913504 --- /dev/null +++ b/.agents/tools/module-split/bisect-member.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash +# Binary-search the smallest number of outlined members of ONE file that makes +# the build ICE. Prints the boundary; member N (1-based) is the trigger. +# +# Usage: bisect_member.sh +set -u +FILE="${1:?file}"; HI="${2:?upper bound}" +lo=0 # known good +hi=$HI # known bad +while [ $((hi - lo)) -gt 1 ]; do + mid=$(( (lo + hi) / 2 )) + git checkout b1563fe -- src/ + git clean -fq src/ + python3 .agents/tools/module-split/split.py --all --write >/dev/null + python3 .agents/tools/module-split/outline.py --write --limit "$mid" "$FILE" >/dev/null + rm -rf target/x86_64-linux-gnu/*/gcm.cache + mcpp build >build/bench/bm.log 2>&1 + ice=$(grep -ac 'internal compiler error' build/bench/bm.log) + err=$(grep -ac ' error: ' build/bench/bm.log) + if [ "$ice" -gt 0 ]; then + echo " limit=$mid ICE -> bad" + hi=$mid + elif [ "$err" -gt 0 ]; then + echo " limit=$mid errors=$err (not an ICE) -> treating as bad" + grep -a ' error: ' build/bench/bm.log | head -2 + hi=$mid + else + echo " limit=$mid clean -> good" + lo=$mid + fi +done +echo "BOUNDARY: $lo good, $hi bad -> member #$hi is the trigger" diff --git a/.agents/tools/module-split/clang-bisect.sh b/.agents/tools/module-split/clang-bisect.sh new file mode 100755 index 00000000..1a142eb8 --- /dev/null +++ b/.agents/tools/module-split/clang-bisect.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# Binary-search the smallest number of outlined members of ONE file that makes +# the clang build fail. Member N (1-based, in outline order) is the trigger. +# +# clang_bisect.sh +set -u +FILE="${1:?file}"; HI="${2:?upper bound}" +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT="$(cd "$HERE/../../.." && pwd)" +cd "$ROOT" || exit 1 +lo=0 +hi=$HI +while [ $((hi - lo)) -gt 1 ]; do + mid=$(( (lo + hi) / 2 )) + out=$(LIMIT=$mid bash "$HERE/clang-variant.sh" "b$mid" "$FILE" 2>&1 | tail -1) + if echo "$out" | grep -q 'rc=0 errors=0'; then + echo " limit=$mid clean -> good" + lo=$mid + else + echo " limit=$mid FAIL -> bad ${out#*errors=}" + hi=$mid + fi +done +echo "BOUNDARY: $lo good, $hi bad -> member #$hi is the trigger" diff --git a/.agents/tools/module-split/clang-variant.sh b/.agents/tools/module-split/clang-variant.sh new file mode 100755 index 00000000..15277235 --- /dev/null +++ b/.agents/tools/module-split/clang-variant.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Build one split variant with clang 20.1.7 (the macOS toolchain family) in an +# isolated copy under build/ (gitignored), so the diagnosis loop is ~40s instead of +# a 13-minute CI cycle. main builds clean under this toolchain, so any failure +# here belongs to the split. +# +# clang_variant.sh