Skip to content

Commit e513bf6

Browse files
The inter-version benchmark reaches two releases further back (#1178)
Two of the five rows in key-commits.txt read "did not build". The reason was not the case being measured: CommonFunctionsInterVersion uses nothing but the public API and compiles against every one of them. It was the other cases in the same project. MatchingEngine reaches `MatchedRules` and `Patterns`, which were internal until v2.3.0, and TransformationLayer names `Transformation.Rationalization`, which did not exist at v2.1.0 -- so the project failed to compile and took the row with it, and the file that would have run was never the problem. `-p:InterVersionOnly=true` compiles the case that runs and leaves the rest out. Measured rather than argued: v2.1.0 and v2.2.0 now build and run, and v2.1.0 produced a full set of figures -- fifteen cases with means and allocations, written to measured-CommonFunctionsInterVersion.json the way every other row is. The unflagged build is unchanged and still compiles every case. The note this replaces said reaching further back would mean conditioning the benchmark project on which commit it is pointed at, and that this would break the comparison because the reason the columns compare is that the benchmark does not vary. The first half was right and the second does not follow. The flag is per-purpose, not per-commit: the script passes it for every row including master, so each column compiles the same files and runs the same file. Conditioning per commit would have broken the comparison; conditioning per purpose does not touch it. Closes #529 -- the measurement half landed in #1140, and this was the limitation recorded against it. The two steps deliberately left out there are unchanged and remain decisions rather than work: the schedule is weekly plus workflow_dispatch rather than daily, and nothing is pushed to a website because neither AngouriMathLab/performance-reports nor its tools repository exists. Claude-Session: https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 5d802ba commit e513bf6

4 files changed

Lines changed: 62 additions & 11 deletions

File tree

‎Sources/Tests/DotnetBenchmark/DotnetBenchmark.csproj‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,30 @@
1212
<AssemblyOriginatorKeyFile>..\..\AngouriMath\key.snk</AssemblyOriginatorKeyFile>
1313
</PropertyGroup>
1414

15+
<!--
16+
Set by `Sources/Utils/benchmark_key_commits.sh`, and by nothing else. The inter-version run
17+
overlays this project onto an old checkout of the kernel, and the cases that reach internals
18+
cannot compile against a kernel that had not made them internal-visible yet: `MatchedRules` and
19+
`Patterns` were internal until v2.3.0, so the whole project failed to build for v2.2.0 and
20+
earlier and those rows read "did not build" — while `CommonFunctionsInterVersion`, the only case
21+
that run executes, uses nothing but the public API and would have compiled.
22+
23+
The flag is per-purpose, not per-commit: the script passes it for *every* row including master,
24+
so what is compiled is the same in each column and the case that runs is the same file. That is
25+
the property the comparison rests on, and conditioning per commit would have broken it.
26+
https://github.com/asc-community/AngouriMath/issues/529
27+
-->
28+
<ItemGroup Condition="'$(InterVersionOnly)' == 'true'">
29+
<Compile Remove="MatchingEngine.cs" />
30+
<Compile Remove="TransformationLayer.cs" />
31+
<Compile Remove="RAMUsageTest.cs" />
32+
<Compile Remove="BenchLinqCompilation.cs" />
33+
<Compile Remove="CacheCompiledFunc.cs" />
34+
</ItemGroup>
35+
<PropertyGroup Condition="'$(InterVersionOnly)' == 'true'">
36+
<DefineConstants>$(DefineConstants);INTERVERSION_ONLY</DefineConstants>
37+
</PropertyGroup>
38+
1539
<ItemGroup>
1640
<None Include="..\..\.editorconfig" Link=".editorconfig" />
1741
<PackageReference Include="BenchmarkDotNet" Version="0.13.0" />

‎Sources/Tests/DotnetBenchmark/Program.cs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,17 @@ public static int Main(string[] args)
7979
.Select(arg =>
8080
arg switch
8181
{
82-
"RAMUsageTest" => GetReportByBenchmark(typeof(RAMUsageTest), "Gen 0", "Gen 1", "Gen 2", "Allocated"),
83-
// Allocated: the class has carried [MemoryDiagnoser] all along, so the
84-
// figure was being collected on every run and then dropped here.
85-
"CommonFunctionsInterVersion" => GetReportByBenchmark(typeof(CommonFunctionsInterVersion), "Mean", "Error", "StdDev", "Allocated"),
82+
// Every case but the inter-version one is compiled out when the project is built for the
83+
// key-commit run, because a case that reaches API an older kernel did not have stops the whole
84+
// project compiling and takes the row with it. Only CommonFunctionsInterVersion is executed by
85+
// that run. https://github.com/asc-community/AngouriMath/issues/529
8686
// Allocated as well as Mean: the regressions this one exists to catch
8787
// show up in allocation and are invisible in the timings.
88+
"CommonFunctionsInterVersion" => GetReportByBenchmark(typeof(CommonFunctionsInterVersion), "Mean", "Error", "StdDev", "Allocated"),
89+
#if !INTERVERSION_ONLY
90+
// Allocated: the class has carried [MemoryDiagnoser] all along, so the
91+
// figure was being collected on every run and then dropped here.
92+
"RAMUsageTest" => GetReportByBenchmark(typeof(RAMUsageTest), "Gen 0", "Gen 1", "Gen 2", "Allocated"),
8893
"TransformationLayer" => GetReportByBenchmark(typeof(TransformationLayer), "Mean", "Error", "StdDev", "Allocated"),
8994
// Ratio as well: this one exists to compare two forms of the same rule,
9095
// and the absolute nanoseconds matter far less than the factor between
@@ -97,6 +102,7 @@ public static int Main(string[] args)
97102
"BenchLinqCompilation" => GetReportByBenchmark(typeof(BenchLinqCompilation), "Mean", "Error", "StdDev", "Allocated"),
98103
"CacheCompiledFunc" => GetReportByBenchmark(typeof(CacheCompiledFunc), "Mean", "Error", "StdDev", "Allocated"),
99104
"NumbersBenchmark" => GetReportByBenchmark(typeof(NumbersBenchmark), "Mean", "Error", "StdDev"),
105+
#endif
100106
_ => throw new($"Unexpected benchmark {arg}")
101107
}).ToArray(); // active action
102108
Console.WriteLine();

‎Sources/Tests/DotnetBenchmark/key-commits.txt‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,21 @@
1919
# is reported as not building rather than skipped, since "this cannot be measured any more" is
2020
# itself worth seeing.
2121
#
22-
# Measured, so the bound is known rather than discovered. At the time of writing v2.3.0 and later
23-
# build; v2.2.0 and earlier do not, because the benchmark's MatchingEngine case reaches
24-
# `MatchedRules` and `Patterns`, which were internal until v2.3.0. Reaching further back means
25-
# compiling only the inter-version case, which means conditioning the benchmark project on which
26-
# commit it is pointed at -- and the whole reason these columns compare is that the benchmark does
27-
# not vary. Left as it is; the table says which rows it could not measure.
22+
# How far back this reaches, and why it is further than it was. v2.3.0 and later used to be the
23+
# bound: v2.2.0 and earlier did not build, because the benchmark's MatchingEngine case reaches
24+
# `MatchedRules` and `Patterns`, which were internal until v2.3.0, and TransformationLayer names
25+
# `Transformation.Rationalization`, which did not exist at v2.1.0.
26+
#
27+
# Neither of those cases is executed by this run. Only CommonFunctionsInterVersion is, and it uses
28+
# nothing but the public API -- so the rows were being lost to files that were compiled and never
29+
# called. `-p:InterVersionOnly=true` compiles the case that runs and leaves the rest out, and the
30+
# script passes it for every row.
31+
#
32+
# The earlier note here said reaching further back would mean conditioning the project on which
33+
# commit it is pointed at, and that this would break the comparison because the benchmark must not
34+
# vary. The first half was right and the second does not follow: the flag is per-purpose, not
35+
# per-commit, so every column compiles the same file and runs the same file. Measured: v2.1.0 and
36+
# v2.2.0 build and run under it.
2837

2938
v2.1.0
3039
v2.2.0

‎Sources/Utils/benchmark_key_commits.sh‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@
1818
# calls the public API as it is now, so an old enough commit genuinely cannot be measured this
1919
# way, and that is worth seeing rather than silently omitting.
2020
#
21+
# What stopped older commits building was not that case, though. It was the *other* cases in the
22+
# same project -- MatchingEngine reaches internals that were internal until v2.3.0, and
23+
# TransformationLayer names Transformation.Rationalization, which did not exist at v2.1.0 -- so
24+
# the project failed to compile and took the row with it while the case that runs would have
25+
# compiled fine. `-p:InterVersionOnly=true` leaves those out. It is passed for every row, so the
26+
# columns still compare: the same file is compiled and the same file runs in each of them.
27+
#
2128
# Run from anywhere.
2229

2330
set -uo pipefail
@@ -60,7 +67,12 @@ for commit in $commits; do
6067
cp -r "$repository/$benchmark" "$tree/Sources/Tests/"
6168
rm -rf "$tree/$benchmark/bin" "$tree/$benchmark/obj" "$tree/$benchmark/benchmark_results.csv"
6269

63-
if ! (cd "$tree/$benchmark" && dotnet build -c Release > "$results/$commit.build.log" 2>&1); then
70+
# InterVersionOnly compiles the one case this run executes and leaves out the rest. A case
71+
# that reaches API an older kernel did not have stops the whole project compiling and takes
72+
# the row with it: MatchedRules and Patterns were internal until v2.3.0, and
73+
# Transformation.Rationalization did not exist until after v2.1.0. Passed for every row
74+
# including master, so what is compiled does not vary between columns.
75+
if ! (cd "$tree/$benchmark" && dotnet build -c Release -p:InterVersionOnly=true > "$results/$commit.build.log" 2>&1); then
6476
echo " did not build — see $results/$commit.build.log"
6577
echo "did not build" > "$results/$commit.status"
6678
continue

0 commit comments

Comments
 (0)