Skip to content

Commit eb166a8

Browse files
committed
fix(bench): the fixture was 74% compiler startup, and the CI workflow never parsed
—— 三个「看起来在测,其实没在测」 **fixture 几乎不含编译。** 实测单个 TU 0.23s,其中 0.17s 是 g++ 启动; 而 `weight` 这个旋钮推不动它 —— 它只产生 O(weight²) 次同一个平凡 constexpr 递归的 实例化(weight=40 也才几百次),编译器微秒级做完。真实对照(gcc 16.1,x86_64): 空模块 .................................. 0.17s 旧 fixture 单元 weight=6 ................ 0.23s ← 74% 是启动 旧 fixture 单元 weight=40 ............... 0.28s ← 6.7× 的旋钮只买到 20% 带真实 global module fragment 的单元 ..... 0.97s mcpp 自己的单元(57k 行 / 139 个)........ 0.57s 只有 units 是线性的(0.088s/个)。**这套东西过去主要在测 g++ 启动。** 工作负载改成真实 C++ 的成本来源:标准库头 + 按**不同类型**实例化 (共用类型的话编译器只实例化一次,后面全免费 —— 这正是旧旋钮失效的原因)。 现在是 `0.38s + 0.066s × weight`,并且有实测扫描钉住:20 units 下 weight 0/4/12 = 4.7s/18.0s/31.4s。默认 weight=4 让单元成本落在 0.64s, 和真实工程同一量级。 **`.github/workflows/bench.yml` 从提交那天起就不是合法 YAML** —— `run: "$BENCH" --list` 被读成一个带引号的标量后面跟垃圾。这个 workflow 一次都没能启动过,而且**没有任何东西会说** :GitHub 仍把坏 workflow 列为 active, `workflow_dispatch`-only 的 workflow 不会被 push 触发,也没有测试看过它。 新增 e2e 232 逐个 parse `.github/workflows/*.yml`,并要求每个都声明了 jobs (能 parse 但没有 jobs 是同一类「静默的什么都不做」)。两侧都验过: 修好的文件通过,坏形态必失败。 **尺寸必须有名字。** 自由三元组 (units, fanin, weight) 无法在两个人之间比较。 加 `--preset smoke|standard|large`,并让**默认形状就等于 standard** —— 否则「没带参数」和「--preset standard」会是两个不同的东西。 bench/README 补成一份真正的规范:§1a 工作负载必须真的是工作负载(新旋钮必须 附实测扫描,否则默认认定为惰性)、§1b 命名尺寸、§4a **有效性规则** (R1 分辨率:落在本引擎 noop 2× 以内的单元测的是进程启动不是构建; R2 离散度:极差/中位数 > 20% 只支持数量级结论)、§4b 明确不做的事、 §4c 采纳了哪些既有实践(SPEC 的全披露与禁止针对性调优、hyperfine 的 预热与离散度报告)以及**这不是什么**(没有审计、单机、跨机器只比表内比值)。 同时修掉一处被自己实测推翻的旧论断:注释里写「GCC 和 Clang 的 BMI 都携带函数体」, 实测 GCC 16.1 **不**携带导出非模板函数的函数体 —— 所以 modules-impl 变体量的是 两种决策规则(比 BMI 内容 vs 信 mtime)的差别,不是编译器限制。 e2e 230 的相对路径检查改成从二进制自身目录运行:relpath 在 Windows 跨盘符 直接抛 `path is on mount 'D:'`,在 macOS 上 `mktemp -d` 给 /var/… 而真实 cwd 是 /private/var/…(深一层)会让 `..` 少一级 —— 这两个 CI 红都是测试自己的缺陷。
1 parent 7313b52 commit eb166a8

6 files changed

Lines changed: 296 additions & 54 deletions

File tree

.github/workflows/bench.yml

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,14 @@ on:
3131
# ever runs — `touch-leaf` was defined, documented and advertised, and
3232
# had never appeared in a single result file.
3333
default: 'cold,noop,touch-hub,touch-leaf,edit-body,edit-comment'
34+
preset:
35+
description: 'named fixture size: smoke | standard | large (overridden by units/fanin/weight below)'
36+
required: false
37+
default: 'standard'
3438
units:
35-
description: 'fixture translation units'
39+
description: 'fixture translation units (0 = use the preset)'
3640
required: false
37-
default: '40'
41+
default: '0'
3842
fanin:
3943
description: 'dependencies per unit (controls graph depth)'
4044
required: false
@@ -122,19 +126,26 @@ jobs:
122126
123127
- name: Report engine availability
124128
shell: bash
125-
run: "$BENCH" --list
129+
run: |
130+
"$BENCH" --list
126131
127132
- name: Run benchmark
128133
shell: bash
129134
run: |
130135
set -euo pipefail
136+
# The preset names the size; units/fanin override it only when set to a
137+
# positive number. Passing raw numbers unconditionally would make every
138+
# run's size an accident of this file rather than a named, comparable
139+
# workload — and --preset must come first so the overrides still win.
140+
args=( --preset "${{ inputs.preset }}" )
141+
[ "${{ inputs.units }}" -gt 0 ] 2>/dev/null && args+=( --units "${{ inputs.units }}" )
142+
[ "${{ inputs.fanin }}" -gt 0 ] 2>/dev/null && args+=( --fanin "${{ inputs.fanin }}" )
131143
"$BENCH" \
132144
--engines '${{ inputs.engines }}' \
133145
--variants '${{ inputs.variants }}' \
134146
--scenarios '${{ inputs.scenarios }}' \
135147
--profile '${{ inputs.profile }}' \
136-
--units '${{ inputs.units }}' \
137-
--fanin '${{ inputs.fanin }}' \
148+
"${args[@]}" \
138149
--runs '${{ inputs.runs }}' \
139150
--work "$RUNNER_TEMP/bench-work" \
140151
--out "bench-${{ matrix.name }}.json"

bench/README.md

Lines changed: 115 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,63 @@ And, orthogonally, **the source form**: the same project emitted three ways.
4141
| `modules` | `unit_k.cppm` declares **and** defines | what most module code looks like |
4242
| `modules-impl` | `unit_k.cppm` declares, `unit_k_impl.cpp` defines | does splitting implementation out of the interface stop edit cascades? |
4343

44-
`modules-impl` exists because of a measured result: on **both** GCC 16.1 and
45-
Clang 22.1 a module interface unit's BMI carries function bodies, so editing any
46-
body changes the BMI and cascades to every importer. No compiler flag fixes it
47-
(`-fmodules-reduced-bmi` was measured and does not). Moving bodies into
48-
implementation units is the only available fix, and this variant is how that
49-
claim gets a number instead of an argument.
44+
`modules-impl` gives the "move bodies out of interface units" advice a number.
45+
What that number is turns out to depend on the compiler, and an earlier version
46+
of this paragraph asserted the **opposite** of the measurement:
47+
48+
* GCC 16.1 does **not** put the body of an exported non-template function into
49+
the BMI. Editing such a body changes the object file and leaves the BMI
50+
byte-identical apart from its embedded timestamps.
51+
* So the cascade other engines pay for that edit is avoidable, and the engines
52+
split by their *decision rule*: compare the BMI's **content** (mcpp, 0.3 s) or
53+
trust its **mtime** (cmake and xmake, ~10 s).
54+
* Templates and inline functions in an interface unit **do** change the BMI. The
55+
advice survives; its justification is narrower than it was written to be.
56+
57+
Establishing this needs a control — compile the *same* source twice and diff the
58+
BMIs. The differing bytes land at the same offsets either way, inside
59+
`buildtime:`/`localtime:`. Without that control the timestamp reads as a content
60+
change and the conclusion inverts.
61+
62+
### 1a. The workload must actually be the workload
63+
64+
A size knob that does not move the cost is worse than no knob: it makes a
65+
benchmark look tunable while it measures something else. The first version of
66+
this fixture failed exactly there.
67+
68+
| | cost per unit (gcc 16.1, x86_64) |
69+
|---|---|
70+
| empty module | 0.17 s |
71+
| **old fixture unit, `weight 6`** | **0.23 s** — 74% of it compiler startup |
72+
| old fixture unit, `weight 40` | 0.28 s — a 6.7x knob bought 20% |
73+
| one unit with a realistic global module fragment | 0.97 s |
74+
| **mcpp's own units** (57k lines / 139 units) | **0.57 s** |
75+
76+
The old `weight` emitted O(weight²) instantiations of one trivial `constexpr`
77+
recursion — a few hundred at weight 40, which a compiler does in microseconds.
78+
Unit *count* scaled cost linearly at 0.088 s each; `weight` did not scale it at
79+
all. **The suite was largely measuring `g++` starting up.**
80+
81+
The workload is now built from what actually costs time in real C++: standard
82+
library headers, plus instantiation over **distinct types** so blocks cannot
83+
share instantiations. Cost is `0.38 s + 0.066 s × weight`, and the knob is
84+
verified to move: at 20 units, `weight` 0 / 4 / 12 gives 4.7 s / 18.0 s / 31.4 s
85+
cold.
86+
87+
**Rule.** Any future knob must come with a measured sweep showing it changes
88+
cost, in this file. A knob without one is assumed inert.
89+
90+
### 1b. Named sizes
91+
92+
A benchmark whose size is a free-form triple of numbers cannot be compared
93+
between two people. `--preset` names it, and the default shape **is** `standard`
94+
so that "no flags" and `--preset standard` cannot mean different things.
95+
96+
| preset | units | fan-in | weight | mcpp cold (gcc, modules) |
97+
|---|---|---|---|---|
98+
| `smoke` | 4 | 2 | 1 | ~2 s — CI and the e2e test, not for publication |
99+
| `standard` | 20 | 3 | 4 | ~18 s — what published results use |
100+
| `large` | 60 | 3 | 6 | minutes |
50101

51102
---
52103

@@ -163,6 +214,64 @@ Two details that are easy to get wrong and change the answer:
163214
the log lives under the WORK root, never inside the measured tree, so a
164215
`--project` run cannot drop scratch into someone's repository.
165216

217+
### 4a. Validity rules — when a cell must NOT be compared
218+
219+
Every result file carries `median_s`, `min_s`, `max_s` and every raw `sample`.
220+
Two rules decide whether a number means anything, and both are computable from
221+
those fields alone — no trust in the harness required.
222+
223+
**R1 — resolution.** Each engine's `noop` row for the same variant is its floor:
224+
what it costs to ask "is anything out of date?" before any work happens. A cell
225+
within **2x of its own engine's `noop`** is measuring process startup and
226+
bookkeeping, not building, and must not be read as a build comparison.
227+
228+
> This is why the `headers` rows read the way they do at small sizes. With the
229+
> old fixture, `cmake` `noop` was 0.33 s and `cmake` `edit-body` was 0.79 s —
230+
> 2.4x, right at the edge. The three fastest engines sat inside a 0.15 s band
231+
> that is *entirely* startup. Those cells were never a ranking.
232+
233+
**R2 — dispersion.** If `(max_s − min_s) / median_s > 0.20`, the cell is noisy
234+
and only order-of-magnitude claims survive it. Report it, do not silently
235+
re-run: a cell that needs re-running to look stable is a cell whose number
236+
depends on the machine's mood.
237+
238+
Neither rule is applied automatically. Automatic suppression hides data; the
239+
rules are stated so a reader applies them, and so a table that violates them is
240+
visibly wrong rather than quietly wrong.
241+
242+
### 4b. What this suite deliberately does not do
243+
244+
* **No CPU pinning, no governor forcing, no `nice`.** Developers do not build
245+
that way. The cost is variance, which R2 exposes rather than hides.
246+
* **No cache dropping.** A cold page cache is not a situation anyone builds in,
247+
and it adds variance unrelated to the engine.
248+
* **No engine-specific tuning.** Each engine gets the same standard, the same
249+
sources, the same compiler binary, the same optimisation level, and whatever
250+
its own documentation says is the normal way to build. Tuning one engine and
251+
not the others is how build-system benchmarks usually go wrong.
252+
* **No confidence intervals.** Run counts are small by necessity; a computed
253+
interval would imply rigour that is not there. Medians with min/max and the
254+
raw samples are what the data supports.
255+
256+
### 4c. Practices this follows, and what it is not
257+
258+
Adopted, with the source of the practice:
259+
260+
| practice | from | here |
261+
|---|---|---|
262+
| full disclosure — host, tool versions, exact command, all flags | SPEC's run rules | §11 + every engine's version recorded by the engine itself |
263+
| no benchmark-specific tuning | SPEC's run rules | §4b |
264+
| warm-up run excluded from the timing | hyperfine, Google Benchmark | one untimed seed build per cell |
265+
| report dispersion, not just a central value | hyperfine | `min_s`/`max_s`/`samples` + R2 |
266+
| distinguish "cannot run" from "ran and failed" || `unavailable` vs `failed`, both requiring a reason |
267+
| a versioned, machine-readable result format || `protocol_version` |
268+
269+
**What this is not.** It is not an audited or certified benchmark, there is no
270+
reviewing body, and the numbers are single-host. Reproducing a published table
271+
requires the same preset, the same engine versions and a comparable machine —
272+
all of which the result file states, which is the point. Treat cross-machine
273+
comparison of absolute seconds as invalid; compare **ratios within one table**.
274+
166275
---
167276

168277
## 5. Declared asymmetries

bench/src/fixture/generate.cppm

Lines changed: 75 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,21 @@
1313
// modules-impl unit_k.cppm declares, unit_k_impl.cpp defines (interface and
1414
// implementation split)
1515
//
16-
// modules-impl exists because of a measured result: with GCC and Clang alike, a
17-
// module interface unit's BMI carries function bodies, so editing ANY body
18-
// cascades to every importer. Moving bodies into implementation units is the
19-
// only fix available (no compiler flag does it — `-fmodules-reduced-bmi` was
20-
// measured and does not). This variant is how that claim gets a number.
16+
// modules-impl exists to give the "move bodies out of interface units" advice a
17+
// number. What that number IS turns out to depend on the compiler, and an
18+
// earlier version of this comment asserted the opposite of the measurement:
19+
//
20+
// * GCC 16.1 does NOT put the body of an exported non-template function into
21+
// the BMI. Editing such a body changes the object file and leaves the BMI
22+
// byte-identical apart from its embedded timestamps, so an engine that
23+
// compares BMI CONTENT correctly rebuilds one unit and stops.
24+
// * An engine that decides from the BMI's mtime cascades anyway, which is why
25+
// cmake and xmake pay ~10 s for that edit where mcpp pays 0.3 s.
26+
//
27+
// So this variant measures the difference between the two decision rules, not a
28+
// compiler limitation. Templates and inline functions in an interface unit are a
29+
// different story and DO change the BMI — the advice survives, its justification
30+
// is narrower than it was written to be.
2131
export module bench.fixture.generate;
2232

2333
import std;
@@ -26,9 +36,16 @@ import bench.protocol;
2636
export namespace bench::fixture {
2737

2838
struct Shape {
29-
int units{40}; // how many translation units
39+
// These defaults ARE the `standard` preset, deliberately: if "no flags" and
40+
// "--preset standard" produced different fixtures, two people comparing
41+
// results would have no way to tell which they each ran.
42+
int units{20}; // how many translation units
3043
int fanin{3}; // how many earlier units each one depends on → graph depth
31-
int weight{6}; // template instantiations per unit → per-unit compile cost
44+
// Distinct template-instantiation blocks per unit. Calibrated, not guessed:
45+
// each block costs ~0.066 s on top of a 0.38 s floor, so 4 puts a unit at
46+
// ~0.64 s — the same order as a real project's units (mcpp's are 0.57 s).
47+
// See detail::support_header() for the measurements behind those numbers.
48+
int weight{4};
3249
};
3350

3451
// Which files a scenario should perturb. The generator knows the shape, so it
@@ -50,42 +67,71 @@ inline std::vector<int> deps_of(int k, const Shape& s) {
5067
}
5168

5269
// Body shared by all three variants, so the WORK is identical and only the
53-
// packaging differs. Templates rather than plain statements: they cost real
54-
// front-end time, which is what a modules benchmark is actually about.
70+
// packaging differs. `weight` blocks, each a DISTINCT template instantiation —
71+
// see support_header() for why distinctness is the whole point, and for what
72+
// one block costs.
5573
inline std::string function_body(int k, const Shape& s) {
5674
std::string b;
5775
b += " long long acc = " + std::to_string(k) + ";\n";
58-
for (int w = 0; w < s.weight; ++w) {
59-
b += std::format(
60-
" acc += ::bench_fixture::mix<{}>(std::tuple<int, long, double>{{{}, {}L, {}.0}});\n",
61-
w, k + w, k * 2 + w, w + 1);
62-
}
76+
for (int w = 0; w < s.weight; ++w)
77+
b += std::format(" acc += ::bench_fixture::work<{}>({});\n", w, k + w);
6378
for (int d : deps_of(k, s))
6479
b += std::format(" acc += {}_value();\n", unit_name(d));
6580
b += " return static_cast<int>(acc & 0x7fffffff);\n";
6681
return b;
6782
}
6883

69-
// The template the bodies instantiate. Header form for the headers variant,
70-
// global-module-fragment form for the module variants — same code either way.
84+
// The template the bodies instantiate. Included by EVERY generated unit — in the
85+
// global module fragment for the module variants, directly for the header
86+
// variant — so all three pay the same per-translation-unit cost and differ only
87+
// in how they share declarations.
88+
//
89+
// CALIBRATION, and why this is not the workload it started as. The first version
90+
// measured almost no compilation: a unit cost 0.23 s of which 0.17 s was the
91+
// compiler starting up — 74% process startup — and the `weight` knob barely
92+
// moved it, because it emitted O(weight^2) instantiations of a single trivial
93+
// constexpr recursion (a few hundred at weight 40, which a compiler does in
94+
// microseconds). Measured on gcc 16.1.0, x86_64:
95+
//
96+
// empty module ................................. 0.17 s
97+
// old fixture unit, weight 6 ................... 0.23 s
98+
// one unit with a realistic global module fragment 0.97 s
99+
// mcpp's own units (57k lines / 139 units) ..... 0.57 s
100+
//
101+
// So the workload is now built from what actually costs time in real C++:
102+
// standard library headers, plus instantiation over DISTINCT types so the
103+
// instantiations cannot be shared between blocks. Cost is 0.38 s + 0.066 s per
104+
// weight unit, which puts the default weight at the same order as a real
105+
// project's units instead of two orders below it.
71106
inline std::string support_header() {
72107
return R"(#pragma once
73-
#include <tuple>
74-
#include <utility>
108+
#include <algorithm>
109+
#include <map>
110+
#include <numeric>
111+
#include <string>
112+
#include <vector>
75113
76114
namespace bench_fixture {
77115
78-
// A small, deliberately template-heavy helper: each instantiation costs the
79-
// front end real work, which is what makes per-unit compile time non-trivial
80-
// enough to measure. Nothing here is meant to be fast at runtime.
81-
template <int N, typename Tuple>
82-
constexpr long long mix(Tuple t) {
83-
if constexpr (N <= 0) {
84-
return static_cast<long long>(std::get<0>(t));
85-
} else {
86-
constexpr std::size_t idx = N % std::tuple_size_v<Tuple>;
87-
return static_cast<long long>(std::get<idx>(t)) + mix<N - 1>(t);
88-
}
116+
// The tag makes every `work<N>` a distinct instantiation of map, vector, string
117+
// and sort. Without it the compiler instantiates one set and every later block
118+
// is free — which is precisely why the previous knob did nothing.
119+
template <int Tag>
120+
struct Key {
121+
int v;
122+
friend bool operator<(const Key& a, const Key& b) { return a.v < b.v; }
123+
};
124+
125+
template <int Tag>
126+
long long work(int seed) {
127+
std::map<Key<Tag>, std::vector<std::string>> m;
128+
for (int i = 0; i < 4; ++i)
129+
m[Key<Tag>{seed + i}].push_back(std::to_string(seed * i));
130+
std::vector<int> v;
131+
v.reserve(8);
132+
for (const auto& [k, strs] : m) v.push_back(static_cast<int>(k.v + strs.size()));
133+
std::sort(v.begin(), v.end());
134+
return std::accumulate(v.begin(), v.end(), 0LL);
89135
}
90136
91137
} // namespace bench_fixture

bench/src/main.cpp

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//
33
// bench [--engines a,b] [--variants headers,modules,modules-impl]
44
// [--scenarios cold,noop,...] [--profile release|debug]
5-
// [--compiler default|gcc|clang] [--units N] [--fanin N] [--weight N]
5+
// [--compiler default|gcc|clang] [--preset NAME] [--units N] [--fanin N] [--weight N]
66
// [--jobs N] [--runs N] [--work DIR] [--out FILE] [--list]
77
//
88
// Writes a protocol-versioned JSON report to --out (default bench-report.json)
@@ -61,9 +61,14 @@ void usage() {
6161
std::println(" --scenarios LIST cold,noop,touch-hub,touch-leaf,edit-body,edit-comment");
6262
std::println(" --profile NAME release | debug (default: release)");
6363
std::println(" --compiler NAME default | gcc | clang (default: default)");
64-
std::println(" --units N fixture translation units (default: 40)");
64+
std::println(" --preset NAME smoke | standard | large — a NAMED size, so two runs on");
65+
std::println(" two machines compare. standard is the default shape.");
66+
std::println(" smoke 4 units / fan-in 2 / weight 1 (~2s, CI)");
67+
std::println(" standard 20 units / fan-in 3 / weight 4 (~18s cold, mcpp)");
68+
std::println(" large 60 units / fan-in 3 / weight 6");
69+
std::println(" --units N fixture translation units (default: 20)");
6570
std::println(" --fanin N dependencies per unit (default: 3)");
66-
std::println(" --weight N template instantiations per unit (default: 6)");
71+
std::println(" --weight N distinct template blocks per unit (default: 4)");
6772
std::println(" --jobs N parallelism handed to each engine (default: engine's)");
6873
std::println(" --runs N repetitions per cell (default: per scenario)");
6974
std::println(" --work DIR scratch directory (default: bench-work)");
@@ -105,6 +110,18 @@ std::expected<Options, std::string> parse(int argc, char** argv) {
105110
else if (a == "--compiler") { auto v = value(a); if (!v) return std::unexpected(v.error()); o.compiler = *v; }
106111
else if (a == "--work") { auto v = value(a); if (!v) return std::unexpected(v.error()); o.work = *v; }
107112
else if (a == "--out") { auto v = value(a); if (!v) return std::unexpected(v.error()); o.out = *v; }
113+
// Presets come FIRST so an explicit --units/--weight after one still
114+
// wins. A benchmark whose size is a free-form pair of numbers cannot be
115+
// compared between two people who ran it; a named size can.
116+
else if (a == "--preset") {
117+
if (i + 1 >= argc) return std::unexpected(std::string("--preset needs a value"));
118+
const std::string_view v = argv[++i];
119+
if (v == "smoke") o.shape = {4, 2, 1};
120+
else if (v == "standard") o.shape = {20, 3, 4};
121+
else if (v == "large") o.shape = {60, 3, 6};
122+
else return std::unexpected(std::format(
123+
"unknown preset '{}' (smoke | standard | large)", v));
124+
}
108125
else if (a == "--units") { if (auto e = take_int(a, o.shape.units)) return std::unexpected(*e); }
109126
else if (a == "--fanin") { if (auto e = take_int(a, o.shape.fanin)) return std::unexpected(*e); }
110127
else if (a == "--weight") { if (auto e = take_int(a, o.shape.weight)) return std::unexpected(*e); }

0 commit comments

Comments
 (0)