Skip to content

Commit 7ca27f3

Browse files
committed
feat(bench): measure the opt-in BMI schedule, and rename the key to say what it does
用户问「cold 怎么没有优化」。因为它确实有 —— gcc 2.29x —— 只是那个开关是被测 **工程 manifest** 里的键,而被测工程现在都是钉住的子模块(其中一个还是别人的 仓库),所以套件根本够不到自己正在测的那个 release 里最大的一项冷构建优化, 表格于是读成「冷构建没有改进」。 mcpp 早就有 `MCPP_BMI_SCHEDULE` 环境变量,所以不需要发明新开关,只需要让 harness 去设它。引擎 spec 加了方括号选项: --engines 'mcpp=<new>,mcpp[schedule=on]=<new>,mcpp' 三条臂进同一份报告、同一个基线、同一台机器、同一分钟。标签自带后缀 (`mcpp@2026.8.13.1+schedule=on`),两行不会并成一行。未知选项**直接拒绝**而不是 静默忽略 —— 你要了选项却量到默认值,正是这套东西要消灭的失败。 **`[build] schedule` 改名为 `[build] bmi_schedule`**(用户提议): * 原来的键只说「调度」,不说调度什么、打开会发生什么; * 而它自己的环境变量一直叫 `MCPP_BMI_SCHEDULE` —— 键与 env 两个拼法不一致; * 下划线也是仓库里的多数写法(9 个下划线键 vs 2 个连字符键)。 它尚未发布、也未进文档,所以改名零成本、不需要别名。 e2e 231 补上**manifest 键本身**的断言:之前只测了 env 那条路径,所以这个键可以 被改名/打错/整个删掉而全部测试照绿 —— 键会静默失效,构建悄悄用回默认值。 xlings 两种代码风格的对照已跑完(gcc,n=1),结果本身就是这次比较的价值: | 场景 | 合并式 old→new | 分离式 old→new | 风格差(new) | |---|---|---|---| | cold | 97.01→92.48s | 29.13→35.88s | **2.58x**(分离式快) | | touch-hub | 89.39→**1.76s** (50.6x) | 24.87→**1.30s** (19.1x) | 1.35x | | edit-body | 89.46→88.33s | 2.73→**1.77s** | **49.96x** | | edit-comment | 95.40→95.02s | 25.09→25.29s | 3.76x | 两个诚实的负面结果,**都还没写进 README,要先复测**: * 分离式的 cold 上新版比旧版**慢 23%**(29.13→35.88s),n=1,可能是噪声也可能 是真回归; * `edit-comment` 在 xlings 上**一点没改善**(而在 mcpp 自己的工程上是 166x)—— xlings 的 platform.cppm 有 56 个函数体,插注释会移动行号,GCC 把内联体的 source location 写进 BMI,所以 BMI 真的变了、级联是对的。fixture 高估了这一项。
1 parent f87c9a0 commit 7ca27f3

12 files changed

Lines changed: 179 additions & 29 deletions

File tree

.agents/docs/2026-08-13-build-optimization-status.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ ninja 不是警告而是**整图拒绝**:
116116
| | 杠杆 | 状态 | 依据 |
117117
|---|---|---|---|
118118
| **L1** | 按次选择工具链 `--toolchain` | **已实施** | 实测 81.8 → **32.6s**(2.51×) |
119-
| **L2** | 下游在 BMI 可用时即开始 | **已实施**(`schedule = "on"`,gcc + clang) | gcc 79.9 → **34.8s**(2.30×);clang 32.0 → **17.95s**(1.78×) |
119+
| **L2** | 下游在 BMI 可用时即开始 | **已实施**(`bmi_schedule = "on"`,gcc + clang) | gcc 79.9 → **34.8s**(2.30×);clang 32.0 → **17.95s**(1.78×) |
120120
| **L3** | 定义移出接口单元 | **不做** —— 已量出它治的是 L2 同一个病 | 实测:对 mcpp **−6.2%**,对 cmake +92.3% |
121121
| **L4** |`build.prepare` | **已实施**(架构收益;性能上为零) | 实测:**0**,原因见下 |
122122

@@ -354,7 +354,7 @@ P1689 扫描的产出上;第一版发射(未提交)会让 `mcpp build` **段错
354354
引擎侧:
355355

356356
* **L1** `--toolchain SPEC` / `MCPP_TOOLCHAIN`:按次选工具链,不动 manifest、不动指纹。
357-
* **L2** `schedule = "on"` / `MCPP_BMI_SCHEDULE`:gcc `detach-codegen` + clang `two-phase`,
357+
* **L2** `bmi_schedule = "on"` / `MCPP_BMI_SCHEDULE`:gcc `detach-codegen` + clang `two-phase`,
358358
决策集中在 `src/build/schedule/policy.cppm`,运行期在 `src/build/schedule/`
359359
默认 `auto` = off。
360360
* **L4** 抽出 `src/build/prepare_inputs.cppm`(架构收益,性能为零 —— 见 §1)。

.github/workflows/bench.yml

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -266,11 +266,29 @@ jobs:
266266
shell: bash
267267
run: |
268268
set -euo pipefail
269+
# The payload has to BE THERE. `mcpp build` installs whatever the
270+
# manifest's default toolchain is and nothing else, so on a clang cell
271+
# the llvm payload may simply not exist — and `--compiler payload:clang`
272+
# is a hard error by design rather than a silent fall back to the host
273+
# clang, which is the failure mode this whole arrangement exists to
274+
# prevent. Install it explicitly, once, before it is asked for.
269275
case "${{ matrix.toolchain }}" in
270-
msvc) echo "BENCH_CXX=msvc" >> "$GITHUB_ENV" ;;
271-
gcc) echo "BENCH_CXX=payload:gcc" >> "$GITHUB_ENV" ;;
272-
clang) echo "BENCH_CXX=payload:clang" >> "$GITHUB_ENV" ;;
276+
msvc)
277+
# No payload: mcpp uses the system Visual Studio (`msvc@system`),
278+
# reached through the VS environment rather than through a path.
279+
echo "BENCH_CXX=msvc" >> "$GITHUB_ENV" ;;
280+
gcc)
281+
"$MCPP" toolchain install "gcc@$(printf '%s' "$TOOLS" | jq -r .gcc)"
282+
echo "BENCH_CXX=payload:gcc" >> "$GITHUB_ENV" ;;
283+
clang)
284+
# Windows is pinned to a different llvm than the other platforms —
285+
# see bench/src/toolchain.cppm, which is where the harness looks it
286+
# up, so the two must name the same version.
287+
if [ "${{ matrix.os }}" = "windows" ]; then key=.llvm_windows; else key=.llvm; fi
288+
"$MCPP" toolchain install "llvm@$(printf '%s' "$TOOLS" | jq -r $key)"
289+
echo "BENCH_CXX=payload:clang" >> "$GITHUB_ENV" ;;
273290
esac
291+
"$MCPP" toolchain list || true
274292
echo "cell compiler: ${{ matrix.toolchain }}"
275293
276294
- uses: ilammy/msvc-dev-cmd@v1
@@ -313,10 +331,19 @@ jobs:
313331
# (i.e. every macOS runner) does not implement, and it fails by NOT
314332
# substituting — the macOS cells would quietly measure one mcpp while
315333
# the Linux ones measured two.
334+
# THREE arms, not two. The third is the same binary with the BMI
335+
# schedule turned on (`[build] bmi_schedule = "on"`), which is opt-in
336+
# until it is verified on every platform — and which the suite could
337+
# not otherwise reach at all, because the switch lives in the MEASURED
338+
# PROJECT's manifest and the measured projects are pinned workloads
339+
# that are not ours to edit. Without it the table said "no improvement
340+
# on cold builds" for the largest cold-build change in the release.
316341
engines=""
317342
IFS=',' read -ra want <<< '${{ matrix.engines }}'
318343
for e in "${want[@]}"; do
319-
if [ "$e" = "mcpp" ]; then e="mcpp=$MCPP_UNDER_TEST,mcpp"; fi
344+
if [ "$e" = "mcpp" ]; then
345+
e="mcpp=$MCPP_UNDER_TEST,mcpp[schedule=on]=$MCPP_UNDER_TEST,mcpp"
346+
fi
320347
engines="${engines:+$engines,}$e"
321348
done
322349
echo "engines: $engines"

bench/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ Four things this bought, each of which had already gone wrong:
107107
> so they are also easy to recognise in a diff.
108108
109109
> **Not exercised by these numbers:** mcpp's split build schedule
110-
> (`[build] schedule = "on"`) is opt-in until it has been verified on every
110+
> (`[build] bmi_schedule = "on"`) is opt-in until it has been verified on every
111111
> platform, so both mcpp binaries run with it off. Its effect is measured
112112
> separately in `.agents/docs/2026-08-13-build-optimization-status.md`.
113113

bench/README.zh-CN.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ job 会打印每个工具实际解析到的版本,与钉的版本不符就大
8686

8787
> **刻意不摁住的**:runner 硬件。见英文版 §4a。
8888
89-
> **这些数字没有覆盖的**:mcpp 的分离式调度(`[build] schedule = "on"`)在所有
89+
> **这些数字没有覆盖的**:mcpp 的分离式调度(`[build] bmi_schedule = "on"`)在所有
9090
> 平台验证通过前是 opt-in 的,所以两个 mcpp 二进制都是关着它跑的。它的效果单独
9191
> 测量,见 `.agents/docs/2026-08-13-build-optimization-status.md`
9292

bench/SPEC.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ the checkout and `reference_mcpp` installed by xlings. Each labels itself from
5656
the version it reports, so the rows never collapse — and the harness warns if
5757
two binaries claim the same version, because then they silently would.
5858

59-
> **Not covered by that column:** the split build schedule (`[build] schedule =
59+
> **Not covered by that column:** the split build schedule (`[build] bmi_schedule =
6060
> "on"`) is opt-in until it has been verified on every platform, so both
6161
> binaries run with it OFF. These numbers therefore do not include it; see
6262
> `.agents/docs/2026-08-13-build-optimization-status.md` for its separately

bench/projects/xlings/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,11 @@ the combined tree, 54 in the split one).
6767

6868
## What it has shown so far
6969

70-
The split module schedule (`schedule = "on"`, see
70+
The split module schedule (`bmi_schedule = "on"`, see
7171
`.agents/docs/2026-08-13-build-performance-architecture.md` L2) reproduces on
7272
both projects, with a *larger* effect on the one that was not used to develop it:
7373

74-
| project | modules / lines | `schedule=off` | `schedule=on` | ratio |
74+
| project | modules / lines | `bmi_schedule=off` | `bmi_schedule=on` | ratio |
7575
|---|---|---|---|---|
7676
| mcpp | 138 / 57k | 79.9s | **34.80s** | **2.30x** |
7777
| **xlings** | 110 / 46k | 112.92s | **33.41s** | **3.38x** |

bench/src/engines/mcpp.cppm

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,24 @@ public:
2626
// `program` may be a bare name resolved through PATH or an absolute path to
2727
// a specific build. `label` is what appears in results; empty means "ask the
2828
// binary", which is what makes a two-version comparison self-describing.
29-
explicit McppEngine(std::string program = "mcpp", std::string label = {})
30-
: program_(std::move(program)), label_(std::move(label)) {}
29+
//
30+
// `env` is how an OPT-IN BEHAVIOUR becomes a measurable engine.
31+
//
32+
// mcpp's split build schedule is `[build] bmi_schedule = "on"` in the measured
33+
// project's manifest, and it is opt-in until it has been verified on every
34+
// platform. That put the benchmark in an impossible position: the manifests
35+
// belong to the pinned workloads (one of them is someone else's project), so
36+
// the suite could not reach the single largest cold-build optimisation in
37+
// the release it was supposed to be measuring — and the table read "no
38+
// improvement on cold builds" for a change worth 2.29x.
39+
//
40+
// mcpp already exposes it as `MCPP_BMI_SCHEDULE`, so no flag had to be
41+
// invented; the harness only had to set it. Setting it per ENGINE rather
42+
// than per run is the point: both arms appear in the same report, against
43+
// the same baseline, on the same machine, in the same minute.
44+
explicit McppEngine(std::string program = "mcpp", std::string label = {},
45+
std::map<std::string, std::string> env = {})
46+
: program_(std::move(program)), label_(std::move(label)), env_(std::move(env)) {}
3147

3248
std::string_view name() const override {
3349
if (label_.empty()) label_ = discover_label();
@@ -53,6 +69,13 @@ public:
5369
platform::RunResult build(const Job& job) const override {
5470
const std::vector<std::string> argv{
5571
program_, "build", job.profile == "debug" ? "--dev" : "--release"};
72+
// Scoped, so the setting reaches THIS engine's child and is restored
73+
// before the next engine runs. A run that leaked it would silently
74+
// measure every later arm with the option on.
75+
std::vector<std::unique_ptr<platform::ScopedEnv>> scoped;
76+
scoped.reserve(env_.size());
77+
for (const auto& [k, v] : env_)
78+
scoped.push_back(std::make_unique<platform::ScopedEnv>(k, v));
5679
return platform::run(argv, job.project_dir, job.log_path, job.timeout_s);
5780
}
5881

@@ -65,8 +88,9 @@ public:
6588
}
6689

6790
private:
68-
std::string program_;
69-
mutable std::string label_;
91+
std::string program_;
92+
mutable std::string label_;
93+
std::map<std::string, std::string> env_;
7094

7195
// `mcpp --version` prints "mcpp <version>". Empty means the binary could not
7296
// be run at all — which probe() reports as unavailable rather than failed.
@@ -84,15 +108,30 @@ private:
84108
if (v.empty()) return "mcpp";
85109
const auto sp = v.rfind(' ');
86110
if (sp == std::string::npos) return "mcpp";
87-
// "mcpp@2026.8.12.1" — distinct per version, so two binaries never
111+
// "mcpp@2026.8.13.1" — distinct per version, so two binaries never
88112
// collapse into one row of the result table.
89-
return std::format("mcpp@{}", v.substr(sp + 1));
113+
//
114+
// The env suffix is part of the identity for the same reason: the SAME
115+
// binary with `schedule=on` is a different engine to measure, and two
116+
// rows called `mcpp@2026.8.13.1` would be unreadable.
117+
std::string out = std::format("mcpp@{}", v.substr(sp + 1));
118+
for (const auto& [k, val] : env_) {
119+
std::string key = k;
120+
// `MCPP_BMI_SCHEDULE` -> `schedule`: the label is read by people.
121+
if (key.starts_with("MCPP_")) key.erase(0, 5);
122+
if (key.ends_with("_SCHEDULE") || key == "BMI_SCHEDULE") key = "schedule";
123+
for (char& c : key) c = static_cast<char>(std::tolower(c));
124+
out += std::format("+{}={}", key, val);
125+
}
126+
return out;
90127
}
91128
};
92129

93130
export std::unique_ptr<Engine> make_mcpp(std::string program = "mcpp",
94-
std::string label = {}) {
95-
return std::make_unique<McppEngine>(std::move(program), std::move(label));
131+
std::string label = {},
132+
std::map<std::string, std::string> env = {}) {
133+
return std::make_unique<McppEngine>(std::move(program), std::move(label),
134+
std::move(env));
96135
}
97136

98137
} // namespace bench::engines

bench/src/registry.cppm

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,68 @@ inline std::string anchor_program(std::string program) {
4646
return ec ? abs.string() : canon.string();
4747
}
4848

49+
// A spec may carry ENGINE OPTIONS in brackets: `mcpp[schedule=on]=/path/to/mcpp`.
50+
//
51+
// This exists for opt-in behaviour. mcpp's split build schedule is a key in the
52+
// MEASURED PROJECT's manifest, and the measured projects are pinned workloads —
53+
// one of them belongs to someone else — so the suite had no way to reach the
54+
// largest cold-build change in the release it was benchmarking, and reported
55+
// "no improvement" for something worth 2.29x.
56+
//
57+
// Bracket options become environment variables for that engine's child only, so
58+
// both arms sit in one report against one baseline on one machine. Unbracketed
59+
// specs are untouched, and an unknown option is an error rather than a silently
60+
// ignored word — a benchmark that quietly measures the default when you asked
61+
// for the option is the exact failure this is meant to remove.
62+
inline std::optional<std::pair<std::string, std::string>> engine_option(
63+
std::string_view engine, std::string_view key, std::string_view value) {
64+
if (engine == "mcpp" && key == "schedule")
65+
return std::pair{std::string("MCPP_BMI_SCHEDULE"), std::string(value)};
66+
return std::nullopt;
67+
}
68+
4969
inline std::unique_ptr<engines::Engine> make_engine(std::string_view spec) {
5070
std::string name(spec);
5171
std::string program;
52-
if (const auto eq = spec.find('='); eq != std::string_view::npos) {
72+
std::map<std::string, std::string> env;
73+
74+
// The BRACKETS are parsed first, then `=program`. Order matters: the option
75+
// list contains `=` itself (`mcpp[schedule=on]=/path`), so splitting on the
76+
// first `=` yields the name `mcpp[schedule`, and the whole spec is rejected
77+
// as an unknown engine.
78+
std::string opts;
79+
if (const auto lb = spec.find('['); lb != std::string_view::npos) {
80+
const auto rb = spec.find(']', lb);
81+
if (rb == std::string_view::npos) return nullptr; // unterminated: reject
82+
name = std::string(spec.substr(0, lb));
83+
opts = std::string(spec.substr(lb + 1, rb - lb - 1));
84+
auto rest = spec.substr(rb + 1);
85+
if (!rest.empty()) {
86+
if (rest.front() != '=') return nullptr; // trailing junk: reject
87+
program = anchor_program(std::string(rest.substr(1)));
88+
}
89+
} else if (const auto eq = spec.find('='); eq != std::string_view::npos) {
5390
name = std::string(spec.substr(0, eq));
5491
program = anchor_program(std::string(spec.substr(eq + 1)));
5592
}
5693

57-
if (name == "mcpp") return engines::make_mcpp(program.empty() ? "mcpp" : program);
94+
{
95+
for (std::size_t at = 0; at <= opts.size();) {
96+
const auto end = std::min(opts.find(',', at), opts.size());
97+
const auto item = std::string_view(opts).substr(at, end - at);
98+
at = end + 1;
99+
if (item.empty()) continue;
100+
const auto sep = item.find('=');
101+
if (sep == std::string_view::npos) return nullptr;
102+
auto mapped = engine_option(name, item.substr(0, sep), item.substr(sep + 1));
103+
if (!mapped) return nullptr; // unknown: reject loudly
104+
env.emplace(std::move(mapped->first), std::move(mapped->second));
105+
}
106+
}
107+
108+
if (name == "mcpp")
109+
return engines::make_mcpp(program.empty() ? "mcpp" : program, {}, std::move(env));
110+
if (!env.empty()) return nullptr; // no other engine takes options yet
58111
if (name == "cmake") return engines::make_cmake();
59112
if (name == "xmake") return engines::make_xmake();
60113
if (name == "bazel") return engines::make_bazel();

src/build/schedule/policy.cppm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ Decision decide(const toolchain::Toolchain& tc, std::string_view requested, int
149149
// not become the default on the strength of one machine. `on` selects it.
150150
if (requested != "on") {
151151
d.reason = "auto: the split schedule is opt-in until it has been "
152-
"verified on every platform (set schedule = \"on\")";
152+
"verified on every platform (set bmi_schedule = \"on\")";
153153
d.ninjaJobs = cap;
154154
return d;
155155
}
@@ -217,7 +217,7 @@ int resolve_jobs(const manifest::Manifest& m,
217217

218218
std::string requested_switch(const manifest::Manifest& m) {
219219
if (const char* e = std::getenv("MCPP_BMI_SCHEDULE"); e && *e) return std::string(e);
220-
if (!m.buildConfig.schedule.empty()) return m.buildConfig.schedule;
220+
if (!m.buildConfig.bmiSchedule.empty()) return m.buildConfig.bmiSchedule;
221221
return "auto";
222222
}
223223

src/manifest/toml.cppm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1046,7 +1046,7 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
10461046
// where they are used, so a bad value warns at build time instead of making
10471047
// the whole manifest unloadable. (A published package carrying an unknown
10481048
// key must never break an older mcpp — same rule the dependency keys follow.)
1049-
if (auto v = doc->get_string("build.schedule")) m.buildConfig.schedule = *v;
1049+
if (auto v = doc->get_string("build.bmi_schedule")) m.buildConfig.bmiSchedule = *v;
10501050
if (auto v = doc->get_string("build.jobs")) m.buildConfig.jobs = *v;
10511051
else if (auto n = doc->get_int("build.jobs")) m.buildConfig.jobs = std::to_string(*n);
10521052
if (auto v = doc->get_string("build.default-profile")) m.buildConfig.defaultProfile = *v;

0 commit comments

Comments
 (0)