Skip to content

Commit bbd9c21

Browse files
committed
fix(bench): stop publishing a build that never happened, and fix the xmake arm's dependency
四个缺陷,都是同一个形状 —— **成功地什么也没做**。 **1. bazel 在 mcpp 工作负载上是个幽灵数字。** `bench/projects/mcpp/BUILD.bazel` 里一条规则都没有(bazel 不能 glob 工作区外 的源码,`import std;` 也没有 bazel 写法),而 `bazel build //...` 对一个**没有 规则的包会退出 0**。于是矩阵发布了: bazel/clang/release/cold/mcpp-2026.8.11.3 0.43s ok 旁边是 mcpp 的 12s 和 cmake 的 94s。每一层都「正确」:bazel 成功了,runner 计了时,报告打印了它。 - 矩阵里把 bazel 从 mcpp 格子移除,理由写进 `excluded`; - 新增 `Engine::unbuildable_reason(Job)` —— `supports()` 只看得到 variant 和 编译器,**看不到工程**,这正是缺口所在。bazel 用 `bazel query kind(rule,//...)` 回答,无规则则报 `unavailable`(有理由)而不是 `ok`(有数字); - e2e 233 加静态守卫:cell 若给 bazel 排了一个无规则的 BUILD.bazel 就红。 **两个方向都钉过**:把 bazel 加回去,守卫确实变红。 **2. 那条「cold 必须 > 自己的 noop 两倍」的不变式抓不到它。** 它是**单引擎内部 的相对判据**,而一个什么都不编的引擎 cold 和 noop 都便宜、比值健康:0.43 对 0.22,以 0.01 秒之差擦过判据。新增**跨引擎**判据:同一 cell 里别的引擎编同一份 源码,快 20 倍就不是引擎快,是工作量小(mcpp 实测最好成绩是 3.1×,阈值远在其外)。 **3. xmake 的 mcpplibs.cmdline 版本是硬编码的,而那是「需求」不是「解析结果」。** `mcpp.toml` 写 `= "0.0.1"` 是**版本要求**;我把它当成了解析结果写死。开发机上 恰好躺着 0.0.1 所以一直是绿的,干净 runner 上 `bench_package_root` 返回 nil, `if` 静默地一个文件都没加,137 个单元之后死于 missing mcpplibs.cmdline dependency for module mcpp.cli.cmd_cache —— 指着消费者,不提版本也不提注册表。改为**读被测树自己的 mcpp.lock**(mcpp 在这棵树上实际做出的解析),两条臂由构造保证编同一份代码;读不到就 `raise`, 不再静默跳过。 ⚠️ 读文件必须放进 `on_load`:描述作用域里 `io` 是 nil。这个坑 `common/xmake/payload.lua` 的注释里白纸黑字写着,我还是又踩了一次。 **4. bench.yml 的 `push:` 没限分支。** 仓库里另外 8 个 workflow 全都是 `branches: [main]`;不限的话,一个开着 PR 的分支会为**同一个 commit** 同时触发 push 和 pull_request 两个 run,而 `concurrency` 去不掉重 —— 两个事件的 `github.ref` 不同(`refs/heads/…` vs `refs/pull/N/merge`)。对一个两小时的矩阵 来说这是最贵的一种重复。 附带:`harness.sh` 的 `$MCPP` 未设置时死在 `line 22: : command not found`, 既不说变量名也不说怎么修。**不回落到 PATH** —— `command -v mcpp` 拿到的是 xlings shim,而 shim 按调用时的工作目录重新挑版本,bench 又刻意在被测树里跑 子进程,于是 cell 报 `mcpp@2026.8.11.2 | unknown command: build`(实测), 和今天早些时候 CI 上参照臂那个缺陷是同一个。改成必须显式给,并把相对路径在 `cd` **之前**转绝对;230 delegator 自己填默认值,这样手跑也成立。
1 parent b85750b commit bbd9c21

11 files changed

Lines changed: 245 additions & 12 deletions

File tree

.github/workflows/bench.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ on:
3737
# workflow's own artifacts land there, so including it would let a results
3838
# commit trigger the run that produces the next results commit.
3939
push:
40+
# main only, matching every other workflow in this repo. Unscoped, a branch
41+
# with an open PR fires BOTH this and the `pull_request` below for the same
42+
# commit — two full matrices, and `concurrency` cannot dedupe them because
43+
# github.ref differs between the events (refs/heads/... vs refs/pull/N/merge).
44+
# For a two-hour job that is the most expensive kind of duplicate.
45+
branches: [main]
4046
paths:
4147
- 'bench/**'
4248
- '!bench/**/*.md'

bench/matrix.json

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@
145145
"os": "linux",
146146
"toolchain": "clang",
147147
"project": "mcpp-2026.8.11.3",
148-
"engines": "mcpp,cmake,xmake,bazel",
148+
"engines": "mcpp,cmake,xmake",
149149
"variants": "modules",
150150
"scenarios": "cold,noop,touch-hub,edit-body,edit-comment",
151151
"hub": "src/platform/platform.cppm",
@@ -156,7 +156,7 @@
156156
"os": "windows",
157157
"toolchain": "clang",
158158
"project": "mcpp-2026.8.11.3",
159-
"engines": "mcpp,cmake,xmake,bazel",
159+
"engines": "mcpp,cmake,xmake",
160160
"variants": "modules",
161161
"scenarios": "cold,noop,touch-hub,edit-body,edit-comment",
162162
"hub": "src/platform/platform.cppm",
@@ -277,6 +277,11 @@
277277
"toolchain": "clang",
278278
"project": "*",
279279
"reason": "KNOWN GAP, and an mcpp/xlings environment defect rather than a benchmark one. On macOS the registry's libc++ ends up on the dynamic loader's search path for EVERY child process, so Apple's own linker — which links against libc++ — resolves against the payload copy and aborts before linking: `dyld: Symbol not found: __ZdaPv, Referenced from: .../XcodeDefault.xctoolchain/usr/bin/ld, Expected in: .../registry/.../lib/libc++.1.0.dylib`. cmake, bazel and the reference mcpp all fail identically while the mcpp under test passes, which is what identifies it as environmental. Same symbol and mechanism as the build.mcpp helper failure in .agents/docs/2026-08-13-build-optimization-status.md S9a. Removing the payload libc++ flags on macOS did NOT fix it, so the contamination arrives through DYLD_* rather than through link flags. The cells return once that is understood — deliberately not guessed at from a machine that cannot reproduce it"
280+
},
281+
{
282+
"project": "mcpp-*",
283+
"engine": "bazel",
284+
"reason": "bazel cannot build this workload: it will not glob sources from outside its workspace, and `import std;` has no bazel spelling. bench/projects/mcpp/BUILD.bazel therefore declares no rules — and `bazel build //...` over a package with no rules EXITS 0 having compiled nothing, which the matrix published as `bazel cold 0.43s` beside mcpp's 12s and cmake's 94s. Removed here, and the bazel adapter now reports `unavailable` for a ruleless package so the next one cannot be published as a measurement."
280285
}
281286
],
282287
"_workload_note": [

bench/projects/mcpp/xmake.lua

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,29 @@ end
6464
MCPP_ROOT = path.normalize(MCPP_ROOT)
6565
local MCPP_MANIFEST = path.join(MCPP_ROOT, "mcpp.toml")
6666

67-
-- mcpp.toml pins mcpplibs.cmdline = "0.0.1" exactly; newer versions may also be
68-
-- unpacked in the registry, so pin rather than take the newest or the two builds
69-
-- would not be compiling the same code.
70-
local CMDLINE_VER = "0.0.1"
71-
local CMDLINE_SRC = bench_package_root("mcpplibs-x-cmdline", CMDLINE_VER)
67+
-- Which version of mcpplibs.cmdline to compile is read from the measured tree's
68+
-- own mcpp.lock, NOT hardcoded and NOT "the newest unpacked".
69+
--
70+
-- * hardcoding it is what broke CI: this said "0.0.1" because mcpp.toml says
71+
-- `mcpplibs.cmdline = "0.0.1"`, but that is a REQUIREMENT, not a resolution.
72+
-- A developer box that had 0.0.1 unpacked from some earlier run worked; a
73+
-- fresh runner had only what mcpp resolved, `bench_package_root` returned
74+
-- nil, the `if` below quietly added no files, and the build died 137 units
75+
-- later with `missing mcpplibs.cmdline dependency for module mcpp.cli` —
76+
-- an error that names neither the version nor the registry.
77+
-- * "the newest unpacked" would silently compile different sources than mcpp
78+
-- did, which is the one thing a comparison arm may not do.
79+
--
80+
-- The lockfile is the resolution mcpp itself performed on this exact tree, so
81+
-- both arms compile the same code by construction.
82+
-- Read inside on_load, not here: `io` is nil in xmake's DESCRIPTION scope, so a
83+
-- reader written at this level dies with `attempt to index a nil value (global
84+
-- 'io')` — the same trap ../common/xmake/payload.lua documents for its manifest
85+
-- reader, walked into a second time. Both the lock path and the registry root
86+
-- are captured as upvalues for the same reason: on_load's sandbox cannot see
87+
-- this file's globals, so `bench_package_root` is not callable from in there.
88+
local MCPP_LOCK = path.join(MCPP_ROOT, "mcpp.lock")
89+
local XPKGS = bench_xpkgs()
7290

7391
option("pin_payload")
7492
set_default(true)
@@ -102,9 +120,39 @@ target("mcpp")
102120
-- has no such cache, so it compiles the 3 units from source. That is a ~1s
103121
-- handicap on xmake's cold build and is called out in the benchmark report
104122
-- rather than hidden.
105-
if CMDLINE_SRC and os.isdir(path.join(CMDLINE_SRC, "src")) then
106-
add_files(path.join(CMDLINE_SRC, "src", "*.cppm"))
107-
end
123+
--
124+
-- Absence is FATAL rather than skipped. Skipping produced a build missing
125+
-- three units out of 140 that announced itself only as
126+
-- `missing mcpplibs.cmdline dependency for module mcpp.cli.cmd_cache` —
127+
-- naming a consumer instead of the cause. A description that cannot name the
128+
-- same sources mcpp compiled is not a comparison arm.
129+
on_load(function (target)
130+
local ver
131+
if os.isfile(MCPP_LOCK) then
132+
local in_section = false
133+
for _, line in ipairs((io.readfile(MCPP_LOCK) or ""):split("\n", {plain = true})) do
134+
local section = line:match("^%s*%[(.-)%]")
135+
if section then in_section = (section == 'package."mcpplibs.cmdline"')
136+
elseif in_section then
137+
ver = ver or line:match('^%s*version%s*=%s*"([^"]+)"')
138+
end
139+
end
140+
end
141+
if not ver then
142+
raise("bench: cannot read mcpplibs.cmdline's resolved version from " .. MCPP_LOCK
143+
.. " — the measured tree must carry the lockfile mcpp resolved it with")
144+
end
145+
local base = path.join(XPKGS, "mcpplibs-x-cmdline", ver)
146+
local dirs = os.isdir(base) and os.dirs(path.join(base, "*")) or {}
147+
table.sort(dirs)
148+
local src = dirs[1] and path.join(dirs[1], "src")
149+
if not src or not os.isdir(src) then
150+
raise("bench: mcpplibs.cmdline " .. ver .. " is not unpacked under " .. XPKGS
151+
.. " — build the tree with mcpp once first, so both arms compile the "
152+
.. "same dependency sources")
153+
end
154+
target:add("files", path.join(src, "*.cppm"))
155+
end)
108156

109157
set_policy("build.c++.modules", true)
110158
set_policy("build.c++.modules.std", true)

bench/src/engines/bazel.cppm

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,34 @@ public:
6262
return {0.0, 0}; // MODULE.bazel/BUILD are the configuration
6363
}
6464

65+
// `bazel build //...` over a package with no rules is a SUCCESS that compiles
66+
// nothing, in about 0.2s. Ask bazel itself what it is about to build rather
67+
// than reading the BUILD file here — a hand-rolled rule detector would be one
68+
// more parser to keep in step with the file it parses.
69+
std::string unbuildable_reason(const Job& job) const override {
70+
platform::RunResult r;
71+
const auto out = platform::run_capture(
72+
{"bazel", "query", "kind(rule, //...)", "--noshow_progress"},
73+
job.buildfile_dir, &r);
74+
// A query that ERRORS is not "no targets" — a broken query is a real
75+
// failure, and it belongs in the build where the log gets reported.
76+
if (!out || !r.ok()) return {};
77+
// The capture is stdout+stderr combined, so presence is tested on the one
78+
// token bazel never prints by accident: a target label at line start.
79+
for (std::string_view rest = *out; !rest.empty();) {
80+
const auto nl = rest.find('\n');
81+
const auto line = rest.substr(0, nl);
82+
if (line.starts_with("//")) return {};
83+
if (nl == std::string_view::npos) break;
84+
rest.remove_prefix(nl + 1);
85+
}
86+
return std::format(
87+
"{}/BUILD.bazel declares no rules, so `bazel build //...` would exit 0 "
88+
"having compiled nothing and report a ~0.2s 'build'. bazel cannot glob "
89+
"sources from outside its workspace and has no spelling for `import std;`",
90+
job.buildfile_dir.filename().string());
91+
}
92+
6593
platform::RunResult build(const Job& job) const override {
6694
std::vector<std::string> argv{"bazel", "build", "//..."};
6795
if (job.jobs > 0) argv.push_back(std::format("--jobs={}", job.jobs));

bench/src/engines/engine.cppm

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,22 @@ public:
5050
// explains itself without a reader consulting this source.
5151
virtual std::string unsupported_reason(Variant v, std::string_view compiler) const = 0;
5252

53+
// "This engine cannot build THIS PROJECT" — the question `supports()` cannot
54+
// ask, because it only sees the variant and the compiler.
55+
//
56+
// The gap was not theoretical. bench/projects/mcpp/BUILD.bazel declares no
57+
// targets at all (bazel will not glob sources from outside its workspace, and
58+
// `import std;` has no bazel spelling), so `bazel build //...` succeeded
59+
// having built nothing, and the cell was published as
60+
// bazel/clang/release/cold/mcpp-2026.8.11.3 0.43s
61+
// next to mcpp's 12s and cmake's 94s. Every layer behaved correctly on its
62+
// own: bazel exited 0, the runner timed it, the report printed it.
63+
//
64+
// Returning a non-empty reason marks the cell `unavailable` — a documented
65+
// gap — instead of `ok` with a number that is off by two orders of magnitude.
66+
// Empty (the default) means "nothing project-specific stops me".
67+
virtual std::string unbuildable_reason(const Job&) const { return {}; }
68+
5369
// Does `compiler` resolve to a clang driver? Several engines' module
5470
// support is clang-only today.
5571
static bool is_clang(std::string_view compiler) {

bench/src/engines/xmake.cppm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public:
5151
// ⚠️ EVERY COMMAND RUNS FROM `buildfile_dir`, i.e. the `-P` directory, and
5252
// that is load-bearing rather than tidiness.
5353
//
54-
// xmake normalises `--buildir` (`-o`) to a path RELATIVE TO THE PROJECT
54+
// xmake normalises `--builddir` (`-o`) to a path RELATIVE TO THE PROJECT
5555
// DIRECTORY, then resolves that relative path against the process's cwd when
5656
// it builds. Run it from anywhere other than `-P` and the two disagree. With
5757
// `-P bench/projects/mcpp` and `-o <workload>/build`, running from

bench/src/main.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,41 @@ int main(int argc, char** argv) {
563563
}
564564
}
565565

566+
// --- cross-engine consistency: a cold build cannot be 20x cheaper than every
567+
// other engine building the same sources ---
568+
//
569+
// The check above is RELATIVE TO ONE ENGINE, so it is blind to the case that
570+
// actually shipped: an engine that builds NOTHING has a cheap cold AND a
571+
// cheap noop, and their ratio looks healthy. bazel on the pinned mcpp tree
572+
// reported cold=0.43s / noop=0.22s — a ratio of 1.95, missing the 2x trip
573+
// wire by one hundredth of a second — while compiling zero of 137 units.
574+
//
575+
// Peers are the honest yardstick here, and they are already in the report:
576+
// engines in the same cell compile the same sources on the same machine, so
577+
// a 20x gap is not a fast engine, it is a different workload. The factor is
578+
// deliberately far past any real result (mcpp's best measured win over cmake
579+
// is 3.1x) so this fires on phantoms and never on a good number.
580+
for (const auto& c : report.cells) {
581+
if (c.status != bench::Status::Ok || c.key.scenario != "cold") continue;
582+
std::vector<double> peers;
583+
for (const auto& p : report.cells)
584+
if (p.status == bench::Status::Ok && p.key.scenario == "cold"
585+
&& p.key.fixture == c.key.fixture && p.key.variant == c.key.variant
586+
&& p.key.engine != c.key.engine && p.median_s() > 0.0)
587+
peers.push_back(p.median_s());
588+
if (peers.empty() || c.median_s() <= 0.0) continue;
589+
std::ranges::sort(peers);
590+
const double peer_median = peers[peers.size() / 2];
591+
if (c.median_s() * 20.0 < peer_median) {
592+
++suspect;
593+
std::println(std::cerr,
594+
"bench: {} reports cold={:.2f}s while other engines building the same "
595+
"sources take {:.2f}s — {:.0f}x is not a faster engine, it is a smaller "
596+
"workload; check that this engine's description actually names the sources.",
597+
c.key.str(), c.median_s(), peer_median, peer_median / c.median_s());
598+
}
599+
}
600+
566601
std::size_t ok = 0, failed = 0, waived = 0;
567602
for (const auto& c : report.cells) {
568603
if (c.status == bench::Status::Ok) { ++ok; continue; }

bench/src/runner.cppm

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,15 @@ public:
288288
job.log_path.filename().string(), tail));
289289
};
290290

291+
// Asked once the Job exists, because the answer depends on the PROJECT —
292+
// which is why it cannot live beside the `supports()` check above.
293+
if (auto why = engine.unbuildable_reason(job); !why.empty()) {
294+
cell.status = Status::Unavailable;
295+
cell.note = std::move(why);
296+
report(cell.note);
297+
return cell;
298+
}
299+
291300
report("configure");
292301
if (const auto cfg = engine.configure(job); !cfg.ok()) {
293302
fail("configure", cfg);

bench/tests/harness.sh

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,35 @@ TMP=$(mktemp -d)
1818
# directory is not a test result.
1919
trap "rm -rf $TMP || true" EXIT
2020

21+
# mcpp's e2e runner exports MCPP as the binary under test; a standalone run has
22+
# nothing to inherit and used to die on `line 22: : command not found`, which
23+
# names neither the variable nor the fix.
24+
#
25+
# NOT defaulted to PATH. `command -v mcpp` returns the xlings SHIM, and a shim
26+
# re-resolves which mcpp to exec from the workspace it is invoked in — while
27+
# bench deliberately runs every engine with its cwd inside the tree under test.
28+
# The shim then picked a different mcpp than the one being tested and the cells
29+
# failed as `mcpp@2026.8.11.2 | unknown command: build`, naming a version nobody
30+
# asked for. This is the same defect as the reference-mcpp arm on CI; the rule
31+
# from bench/README §4 is absolute: an engine is named by BINARY, never by PATH.
32+
[ -n "${MCPP:-}" ] || {
33+
echo "FAIL: MCPP is unset. Set it to a REAL mcpp binary, not a PATH name:"
34+
echo " MCPP=\$(bash .github/tools/newest_artifact.sh . mcpp) bash bench/tests/harness.sh"
35+
echo " (mcpp's e2e runner exports it; \`bash tests/e2e/230_bench_harness.sh\` works too.)"
36+
exit 1
37+
}
38+
# Absolutised against the INVOKING cwd, which is why this runs before the `cd`
39+
# below: `MCPP=./target/.../mcpp` is what every natural way of producing the
40+
# path yields, and a relative one silently stops resolving the moment the script
41+
# changes directory — including the command this very message suggests.
42+
case "$MCPP" in
43+
/*|?:[/\\]*) ;;
44+
*) MCPP="$PWD/$MCPP" ;;
45+
esac
46+
[ -x "$MCPP" ] || { echo "FAIL: MCPP=$MCPP is not an executable file"; exit 1; }
47+
2148
cd "$REPO/bench"
49+
echo "harness built with: $("$MCPP" --version 2>&1 | head -1) ($MCPP)"
2250
"$MCPP" build > /dev/null
2351

2452
# NEWEST, not `find | head -1`: target/ holds one directory per toolchain

tests/e2e/230_bench_harness.sh

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,19 @@
77
# directory would make that separation a rename away from breaking.
88
#
99
# This delegator stays because deleting it would silently drop the harness from
10-
# every mcpp PR: `bench.yml` is workflow_dispatch-only, so nothing else runs it.
10+
# every mcpp PR that does not touch bench/: `bench.yml` is PATH-SCOPED to
11+
# `bench/**`, so a change elsewhere that breaks the suite runs nothing.
1112
set -e
1213
REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
14+
15+
# The e2e runner exports MCPP as the binary under test and that always wins.
16+
# Filling it in when unset is what makes this script runnable BY HAND, which the
17+
# harness cannot do for itself: it must not fall back to PATH (that resolves to
18+
# the xlings shim, which re-picks a version per working directory), but this
19+
# delegator lives in mcpp's own tree and can simply point at what was built.
20+
if [ -z "${MCPP:-}" ]; then
21+
MCPP="$(bash "$REPO/.github/tools/newest_artifact.sh" "$REPO" mcpp 2>/dev/null || true)"
22+
[ -n "$MCPP" ] || { echo "SKIP: no mcpp binary built yet — run \`mcpp build\` first"; exit 0; }
23+
export MCPP
24+
fi
1325
exec bash "$REPO/bench/tests/harness.sh"

0 commit comments

Comments
 (0)