Skip to content

Commit 66007af

Browse files
Fail the build when the popular use cases allocate more (#529, #500) (#1014)
The benchmark job measured parse, Simplify, Solve and Differentiate on every push to the kernel and then uploaded the numbers, which meant a regression was visible to whoever went looking and to nobody else. #746's standing condition is that speed and memory on these paths are measured rather than hoped for, and a measurement that nothing fails on is not that. PerformanceGate compares a run against Sources/Tests/DotnetBenchmark/performance-baseline.json and returns a non-zero exit code. It gates allocation and not the wall clock, because only one of the two is a property of the code: allocated bytes per operation is what the program asked the allocator for, and the same input asks for the same number of bytes, while a GitHub-hosted runner's wall clock is a property of whoever else is on that host. Measured before choosing the thresholds -- all eighteen benchmarks, three consecutive runs of one unchanged build, on a machine deliberately left loaded -- allocation was at most 0.033% apart run to run on the sixteen gated benchmarks and identical to the byte on seven of them, while the mean time was up to 51.8% apart and 8-10% apart on nine of the eighteen. Three orders of magnitude between the two is the argument: allocation is held to 3%, a hundred times the observed spread, and the wall clock fails only above 3x, which is a catastrophe threshold and deliberately catches nothing smaller. CompileEasy and CompileHard are excluded by name, at 1.0% and 3.6% over those same three runs, because what they measure includes the runtime building and JIT-compiling a delegate. The exclusion lives in the baseline's ungated map with its reason and the gate prints it on every run, so it is a decision somebody made rather than a benchmark missing from the file. It fails on an improvement too, as the corpus gate does and for the same reason: a baseline that overstates what the library allocates would let a later change give the gain back unnoticed. The verdict says IMPROVED rather than REGRESSED and says to update the baseline. The run writes its numbers in the baseline's own format, so updating the baseline is one file copied over another -- including from the artifact of the job that just failed, which is why the upload step keeps if: always() and why the gate runs before it. #529 #500 #746 Claude-Session: https://claude.ai/code/session_011emtnRT6EWTrxXNtqDVK3e Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ebf8475 commit 66007af

6 files changed

Lines changed: 697 additions & 5 deletions

File tree

‎.github/workflows/Benchmark.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,22 @@ jobs:
5555
cd Sources/Tests/DotnetBenchmark
5656
dotnet run -c Release RAMUsageTest
5757
58+
# The step that makes the two above a gate rather than a report. It runs no benchmark:
59+
# the CPU step wrote what it measured to benchmark_results.csv/measured-*.json, and this
60+
# reads that against the committed performance-baseline.json. Allocation is what it fails
61+
# on, in either direction; the wall clock is reported and only fails at 3x, because a
62+
# GitHub-hosted runner shares its host and a tight time threshold would be red for
63+
# reasons that are not the change. The argument, and the variance it was measured from,
64+
# are in PerformanceGate.cs.
65+
#
66+
# Last, so that a failure here does not cost the RAM figures, and before the upload so
67+
# that the file to copy over the baseline is in the artifact of the run that failed.
68+
# https://github.com/asc-community/AngouriMath/issues/529
69+
- name: 'Kernel performance gate'
70+
run: |
71+
cd Sources/Tests/DotnetBenchmark
72+
dotnet run -c Release --no-build PerformanceGate
73+
5874
# The numbers used to exist only in this job's log, which expires -- so there was no way to
5975
# see whether a figure had moved without re-running an old commit by hand. Retaining them is
6076
# step 1 of https://github.com/asc-community/AngouriMath/issues/500, and the only step that

‎AGENTS.md‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,26 @@ suite is paid for on every commit. The harnesses in `work/` are where a measurem
200200
inputs, takes minutes, and gets read by a person; the two are not substitutes, and a finding from a
201201
harness that is worth keeping belongs in the corpus as a new problem.
202202

203+
### And a second gate, on what the popular use cases allocate
204+
205+
`Sources/Tests/DotnetBenchmark/performance-baseline.json` records allocated bytes and mean
206+
nanoseconds per operation for parse, `Simplify`, `Solve` and `Differentiate` on textbook-sized input.
207+
The Kernel Benchmark workflow runs the benchmarks and then `PerformanceGate`, which compares the run
208+
against that file and fails the build. It behaves like the corpus gate — including failing when a
209+
number gets **better**, because a baseline that overstates what the library allocates would let a
210+
later change give the gain back unnoticed.
211+
212+
**It gates allocation and reports time.** Allocated bytes are a property of the code: the same input
213+
asks the allocator for the same number of bytes. A shared runner's wall clock is a property of
214+
whoever else is on that host, so the time column fails only above 3x — a catastrophe threshold, not
215+
a performance one. The measurement that chose those two numbers is in the remarks of
216+
`PerformanceGate.cs`.
217+
218+
It runs on the benchmark job rather than in the suite, because it costs about ten minutes and
219+
everything in the suite is paid for on every commit. What to do when it fails, and when updating the
220+
baseline is legitimate, is in
221+
[`WhatsNew/version_performance_control.md`](Sources/AngouriMath/Docs/WhatsNew/version_performance_control.md).
222+
203223
## One structure under several features
204224

205225
Four things in this library are the same shape, and three of them were written separately before
@@ -400,7 +420,7 @@ are short, and a stale one is worse than none — if you change what a file desc
400420
| [`Contributing/SimplificationContract.md`](Sources/AngouriMath/Docs/Contributing/SimplificationContract.md) | what a rewrite may assume, and the ten obligations one has to meet. Read it *before* adding or changing a rule |
401421
| [`Contributing/CanonicalForm.md`](Sources/AngouriMath/Docs/Contributing/CanonicalForm.md) | canonical versus simplest, why no canonical form exists for the whole language, and what one means per node class. Read it before comparing two expressions for equality |
402422
| [`Contributing/coding_rules.md`](Sources/AngouriMath/Docs/Contributing/coding_rules.md) | sealed-or-abstract, and immutability of `Entity` |
403-
| [`WhatsNew/version_performance_control.md`](Sources/AngouriMath/Docs/WhatsNew/version_performance_control.md) | the inter-version performance table, and how to add a column |
423+
| [`WhatsNew/version_performance_control.md`](Sources/AngouriMath/Docs/WhatsNew/version_performance_control.md) | the inter-version performance table, how to add a column, and when the CI performance baseline may be updated |
404424
| `Sources/Analyzers/` | the custom analyzers, including the static-field one behind `[ConstantField]` |
405425

406426
Anything added for the library's own purposes is not `public` — see
@@ -434,7 +454,8 @@ whatever else it delivered:
434454
[`WhatsNew/version_performance_control.md`](Sources/AngouriMath/Docs/WhatsNew/version_performance_control.md).
435455
Measure the previous column again on the same machine and publish the pair: columns taken on
436456
different hardware cannot be read as a ratio, and a uniform factor across every row is the machine
437-
rather than the code.
457+
rather than the code. Since #529 that is a gate and not only a record — see *And a second gate, on
458+
what the popular use cases allocate* above.
438459
3. **Correctness coverage grows with the surface.** Each new layer adds ways to be wrong that the one
439460
below could not express.
440461

‎Sources/AngouriMath/Docs/WhatsNew/version_performance_control.md‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,82 @@ take a millisecond in total, while a thousand evaluations of freshly built nodes
4747
The row is kept as it is so the column history stays comparable, and the two trigonometric
4848
benchmarks build their expression afresh on every call for exactly this reason.
4949

50+
---
51+
52+
## The gate, and when the baseline may be updated
53+
54+
This file is a history a person reads. `Sources/Tests/DotnetBenchmark/performance-baseline.json`
55+
is the same measurement in the form a build can fail on, which is what
56+
[#746](https://github.com/asc-community/AngouriMath/issues/746)'s standing condition -- *speed and
57+
memory on popular use cases are measured, not hoped for* -- asks for and what
58+
[#529](https://github.com/asc-community/AngouriMath/issues/529) asked for before it. The Kernel
59+
Benchmark workflow runs the benchmarks and then `dotnet run -c Release PerformanceGate`, which
60+
compares that run against the committed file.
61+
62+
**It gates allocation and not time, because only one of the two is a property of the code.**
63+
Allocated bytes per operation is what the program asked the allocator for: the same code on the
64+
same input asks for the same number of bytes. Wall-clock time on a GitHub-hosted runner is a
65+
property of whoever else is on that host. Measured before choosing the thresholds -- all eighteen
66+
benchmarks, three consecutive runs of one unchanged build, on a machine deliberately left loaded
67+
(eight cores, load average around 16, which is closer to a shared runner than an idle desktop is):
68+
69+
| | run to run |
70+
|---|---|
71+
| allocation, the sixteen gated benchmarks | at most **0.033%** apart -- six bytes on eighteen kilobytes, for `ParseEasy` -- and identical to the byte on seven of them |
72+
| mean time | up to **51.8%** apart, and 8-10% on nine of the eighteen |
73+
74+
Three orders of magnitude between the two is the whole argument. Allocation is held to 3%, a
75+
hundred times the observed spread, and the wall clock fails only above 3.00x, which is a
76+
catastrophe threshold and deliberately catches nothing smaller. A gate that is red for reasons that
77+
are not the change is a gate people learn to ignore, and that is worse than not having one.
78+
79+
**Two benchmarks are excluded by name.** `CompileEasy` and `CompileHard` moved 1.0% and 3.6% over
80+
those same three runs, because what they measure includes the runtime building and JIT-compiling a
81+
delegate, which is not reproducible the way the library's own allocation is. They are listed in the
82+
baseline's `ungated` map with that reason, and the gate prints the reason every time it runs -- an
83+
exclusion is a decision somebody made, not a benchmark that happens to be missing from the file.
84+
85+
**It fails when a number gets better, too**, exactly as the corpus gate does and for the same
86+
reason: a baseline that overstates what the library allocates silently permits giving the gain
87+
back. The failure says `IMPROVED` rather than `REGRESSED`, and says what to do about it.
88+
89+
### How to update it
90+
91+
The run writes its numbers in the baseline's own format, so updating the baseline is a copy rather
92+
than a transcription:
93+
94+
```
95+
cd Sources/Tests/DotnetBenchmark
96+
dotnet run -c Release CommonFunctionsInterVersion
97+
cp benchmark_results.csv/measured-CommonFunctionsInterVersion.json performance-baseline.json
98+
dotnet run -c Release --no-build PerformanceGate
99+
```
100+
101+
The same file is in the `benchmark-results-<sha>` artifact of the CI job -- including of the job
102+
that just failed, which is why the upload step runs `if: always()`. That copy is the better source
103+
of the two, because it was taken on the machine and the runtime the gate is checked against.
104+
105+
### When updating it is legitimate
106+
107+
- **The allocation went down.** Record it, in the same change. This is the case the gate exists to
108+
stop being absorbed, and it is the only one where a red build means nothing is wrong.
109+
- **The allocation went up and buys something.** [#918](https://github.com/asc-community/AngouriMath/pull/918)
110+
is the worked example, below in this file: the polynomial layer made `SolveMedium` allocate 19%
111+
more and made an incomplete root set complete. Say which in the pull request, next to the
112+
`BREAKING-CHANGES.md` entry that a changed answer already owes.
113+
- **A benchmark was added or removed.** The gate reports `NEW` or `MISSING`, and the baseline
114+
follows in the same change.
115+
- **The runtime moved under it.** Allocation is a property of the code *and* of the BCL beneath it,
116+
so a .NET version bump can move a row with nothing here having changed. The baseline records the
117+
`runtime` and `machine` it was taken on, and the gate prints both against the run's own, so check
118+
those two lines before concluding this -- and say so in the commit, because it is the one reason
119+
on this list that leaves no trace in the diff.
120+
121+
And when it is not: **because the build is red and the number is inconvenient.** The gate names the
122+
benchmark and the size of the move. A move nobody can explain is the finding, not the obstacle.
123+
124+
---
125+
50126
| Method | [331st](https://github.com/asc-community/AngouriMath/commit/10e6e5a90e7270336b68dc5fd6aa36f3e0e65d2b) | [380th](https://github.com/asc-community/AngouriMath/commit/73ae36488ddb863c1d6f35db5ed2f5dcf1484a26) | [391st](https://github.com/asc-community/AngouriMath/commit/c7e08e6936bfdc2373377bec81ffd160e406244f) | [410th](https://github.com/asc-community/AngouriMath/commit/20814936bc740a9f410af4a4368e9895eab7aaf7) | [483rd](https://github.com/asc-community/AngouriMath/commit/355963dcdf0ff9da568e9f1144ad2b7b68c19584) | [520th](https://github.com/asc-community/AngouriMath/commit/70aa71acb73307c9f7df0aac006faae31b06058c) | [690th](https://github.com/asc-community/AngouriMath/commit/5cc894939cb3657f0aa7ef5a25fd55011058929f) | [826th](https://github.com/asc-community/AngouriMath/commit/87e33ec3590a95dd4ec59ff5c1f77064a64196d1) | [914th](https://github.com/asc-community/AngouriMath/commit/6134338df083a908369b6bcfb69e70a4269ec51b) | [920th](https://github.com/asc-community/AngouriMath/commit/501a0a3a9b2e07cddf92c4446b73ad6e2748253a) | [1034th](https://github.com/asc-community/AngouriMath/commit/a3f48b47795b2dc2b3435152989a6e15639a65b4) | [1066th](https://github.com/asc-community/AngouriMath/commit/a33746651f56a380b6c17913aa844f162f258d8c) | [1090th](https://github.com/asc-community/AngouriMath/commit/9530c5b04484e98023941f7693bbeb1a3282cee6) | [1446th](https://github.com/asc-community/AngouriMath/commit/2abb2b537c03977281f3fc2cab1da2c78c36a5f5) | [1620th](https://github.com/asc-community/AngouriMath/commit/e05d71797f53a9ac7dbdad6075cd7302a91036e7) | [1671st](https://github.com/asc-community/AngouriMath/commit/253b5a8d598a9a1d721b777340bd53ae3be12f99) | [1769th](https://github.com/asc-community/AngouriMath/commit/3ac24bc241e898509533bc980ec5dc91c5d79a07) |
51127
|---------------- |--------------:|---------------:|---------------:|--------------:|----------------:|--------------:|--------------:|--------------:|--------------:|--------------:|--------------:|--------------:|--------------:|-----------------:|--------------:|--------------:|--:|
52128
| ParseEasy | 28,599 | 73,669 | 134,120 | 44,328 | 54,675 | 21,722 | 32,212 | 32,138 | 34,702 | 32,199 | 33,008 | 27,483 | 33,043 | 34,664 | 11,760 ns | 10,857 ns | 10,080 ns |

0 commit comments

Comments
 (0)