[Git][ghc/ghc][wip/sjakobi/T27653] 2 commits: testsuite: Migrate perf tests off collect_compiler_stats('all')
Simon Jakobi pushed to branch wip/sjakobi/T27653 at Glasgow Haskell Compiler / GHC Commits: 1324f1f0 by Simon Jakobi at 2026-08-08T15:39:20+02:00 testsuite: Migrate perf tests off collect_compiler_stats('all') The 'all' metric argument applies a single tolerance to bytes allocated, max_bytes_used and peak_megabytes_allocated, whose noise profiles are incompatible (#27653): allocations are nearly deterministic, residency needs 10-20%, and peak is quantized to 1 MB. Any single tolerance is too tight for one metric or too slack for another. This migrates all users of 'all' (and of the 'all' default) to explicit per-metric collection, ahead of removing 'all' from the driver. peak_megabytes_allocated is dropped everywhere: its 1 MB granularity makes tight relative windows meaningless (#27613), and it is sensitive to GC timing -- in #27489 it drifted by -5.3% while max_bytes_used moved by less than 0.1%. Where a test guards a memory property, max_bytes_used covers it at byte granularity. Where the motivating ticket was about compile-time memory (T11545, T15304, T26425, LinkableUsage01/02), residency remains gated via max_bytes_used, now with a residency-appropriate tolerance. max_bytes_used is dropped where residency was only ever an accident of 'all': * T15630, T15630a, T20261: the underlying tickets (#15630, #20261) contain no memory data at all -- one is a simplifier-ticks blowup, the other is stated entirely in allocation numbers -- and the 20% window never had teeth. * T21839c: #21839's measurements show residency essentially flat (+0.16%) while allocations moved +7%, so allocations are the discriminating metric; they are already gated at 1% via collect_compiler_runtime. The ghc/max gate has previously broken CI spuriously (widened 1% -> 10% in 9fd11585eb for that reason). Allocation tolerances are tightened to the testsuite's conventional 2% where 'all' previously left them at 10-20%. Assisted-by: Claude Fable 5 - - - - - b28f7e48 by Simon Jakobi at 2026-08-08T15:39:20+02:00 testsuite: Remove the 'all' metric argument of collect_stats 'all' (also the default) gated bytes allocated, max_bytes_used and peak_megabytes_allocated at a single tolerance, although their noise profiles are incompatible, making such tests either flaky or toothless (#27653). Metric collection composes -- one collect_stats call per metric -- so 'all' had no use beyond the footgun. All users were migrated in the previous commit; this removes the argument and the default from the driver and updates the documentation accordingly. Assisted-by: Claude Fable 5 - - - - - 5 changed files: - testsuite/driver/README.md - testsuite/driver/testlib.py - testsuite/tests/bytecode/TLinkable/all.T - testsuite/tests/perf/compiler/all.T - testsuite/tests/perf/space_leaks/all.T Changes: ===================================== testsuite/driver/README.md ===================================== @@ -35,11 +35,13 @@ differences, and things such as that are not necessary to be considered by the test writer anymore. This is due to the fact that the test comparison relies entirely on locally collected metrics on the testing machine. -As such, it is perfectly sufficient to write `collect_stats('all',20)` in the -".T" files to measure the 3 potential stats that can be collected for that test -and automatically test them for regressions, failing if there is more than a 20% -change in any direction. In fact, even that is not necessary as -`collect_stats()` defaults to 'all', and 20% deviation allowed. +A test states which metric to measure and how much deviation to allow, e.g. +`collect_stats('bytes allocated', 2)` in the ".T" files; the test fails if the +metric changes by more than 2% in either direction. To gate several metrics, +use one `collect_stats` call per metric, so that each gets a tolerance matched +to its noise profile: allocations are nearly deterministic and support tight +windows, while the residency metrics need considerably slacker ones (see +Note [Measuring residency] in testlib.py). The function `collect_compiler_stats()` is completely equivalent in every way to `collect_stats` except that it measures the performance of the compiler itself ===================================== testsuite/driver/testlib.py ===================================== @@ -24,7 +24,7 @@ import subprocess from testglobals import config, ghc_env, default_testopts, brokens, t, \ TestRun, TestResult, TestOptions, PerfMetric from testutil import strip_quotes, lndir, link_or_copy_file, passed, \ - failBecause, testing_metrics, residency_testing_metrics, \ + failBecause, residency_testing_metrics, \ stable_perf_counters, \ PassFail, badResult, str_warn, str_removeprefix from term_color import Color, colored @@ -816,28 +816,20 @@ def _collect_generic_stat(name : TestName, opts, metric_infos): # ----- -# Defaults to "test everything, and only break on extreme cases" -# -# The inputs to this function are slightly interesting: -# metric can be either: -# - 'all', in which case all 3 possible metrics are collected and compared. -# - The specific metric one wants to use in the test. -# - A set of the metrics one wants to use in the test. -# -# Deviation defaults to 20% because the goal is correctness over performance. -# The testsuite should avoid breaking when there is not an actual error. -# Instead, the testsuite should notify of regressions in a non-breaking manner. +# metric is either a single metric name or a set of metric names. Use one +# call per metric so each gets a tolerance matched to its noise profile; +# see Note [Measuring residency] for the residency metrics. # # collect_compiler_stats is used when the metrics collected are about the compiler. # collect_stats is used in the majority case when the metrics to be collected # are about the performance of the runtime code generated by the compiler. -def collect_compiler_stats(metric='all',deviation=20): +def collect_compiler_stats(metric, deviation=20): def f(name, opts, m=metric, d=deviation): no_lint(name, opts) return _collect_stats(name, opts, m, d, None, True) return f -def collect_stats(metric='all', deviation=20, static_stats_file=None): +def collect_stats(metric, deviation=20, static_stats_file=None): return lambda name, opts, m=metric, d=deviation, s=static_stats_file: _collect_stats(name, opts, m, d, s) def statsFile(comp_test: bool, name: str) -> str: @@ -864,12 +856,9 @@ def _collect_stats(name: TestName, opts, metrics, deviation: Optional[int], # This is a bit weird, though. return - # Normalize metrics to a list of strings. + # Normalize metrics to a set of strings. if isinstance(metrics, str): - if metrics == 'all': - metrics = testing_metrics() - else: - metrics = { metrics } + metrics = { metrics } opts.is_stats_test = True if is_compiler_stats_test: ===================================== testsuite/tests/bytecode/TLinkable/all.T ===================================== @@ -3,7 +3,8 @@ # after they have been loaded into the `LoaderState`. # However, this property is currently not validated automatically. test('LinkableUsage01' - , [ collect_compiler_stats('all', 2) + , [ collect_compiler_stats('bytes allocated', 2) + , collect_compiler_stats('max_bytes_used', 5) , extra_files(['genLinkables.sh', 'BCOTemplate.hs']) , pre_cmd('$MAKE -s --no-print-directory LinkableUsage01_Prep') , req_bco @@ -18,7 +19,8 @@ test('LinkableUsage01' # Performance test for bytecode `Linkable`s. test('LinkableUsage02' - , [ collect_compiler_stats('all', 2) + , [ collect_compiler_stats('bytes allocated', 2) + , collect_compiler_stats('max_bytes_used', 5) , extra_files(['genLinkables.sh', 'BCOTemplate.hs']) , pre_cmd('$MAKE -s --no-print-directory LinkableUsage02_Prep') , req_bco ===================================== testsuite/tests/perf/compiler/all.T ===================================== @@ -640,13 +640,16 @@ test ('T15164', ], compile, ['-v0 -O']) +# T15630/T15630a guard against exponential simplifier blowup when inlining +# join points (#15630). T15630a is a monomorphic variant that has blown up +# even when T15630 was fine; see the comment in T15630.hs. test('T15630', - [collect_compiler_stats() + [collect_compiler_stats('bytes allocated', 2) ], compile, ['-O2']) test('T15630a', - [collect_compiler_stats() + [collect_compiler_stats('bytes allocated', 2) ], compile, ['-O2']) @@ -770,12 +773,19 @@ test ('T9198', compile, ['']) +# Guards against quadratic demand-analysis cost on wide recursive data +# types, which manifested as a compile-time memory blowup (#11545). test('T11545', - [ collect_compiler_stats('all', 15) ], + [ collect_compiler_stats('bytes allocated', 2) + , collect_compiler_residency(15) ], compile, ['-O']) +# Guards against the compile-time memory blowup of #15304, caused by +# over-keen inlining and demand-analysis memory usage on a module with +# many wide strict constructors. test('T15304', - [ collect_compiler_stats('all', 10) ], + [ collect_compiler_stats('bytes allocated', 2) + , collect_compiler_residency(10) ], compile, ['-O']) test ('T20049', [ collect_compiler_stats('bytes allocated',2) ], @@ -797,8 +807,10 @@ test('T16875', # Testing one hole-fit with a lot in scope for #16875 collect_compiler_runtime(4), compile, ['-fdefer-type-errors -fno-max-valid-hole-fits -package ghc']) +# Guards against renamer/typechecker allocation regressions on very large +# generated modules (#20261, a Happy-generated parser). test ('T20261', - [collect_compiler_stats('all')], + [collect_compiler_stats('bytes allocated', 2)], compile, ['']) @@ -807,8 +819,7 @@ test ('T20261', # does not sensibly handle one test acting as both # a compile-time and a run-time performance test test('T21839c', - [ collect_compiler_stats('all', 10), - collect_compiler_runtime(1), + [ collect_compiler_runtime(1), only_ways(['normal'])], compile, ['-O']) @@ -874,8 +885,12 @@ test('interpreter_steplocal', ghci_script, ['interpreter_steplocal.script']) +# Guards against the compile-time memory blowup of #26425; primarily +# stresses OccAnal and unfolding performance on a long chain of nested +# join points and cases. test ('T26425', - [ collect_compiler_stats('all',20) ], + [ collect_compiler_stats('bytes allocated', 2) + , collect_compiler_residency(20) ], compile, ['-O']) ===================================== testsuite/tests/perf/space_leaks/all.T ===================================== @@ -1,9 +1,6 @@ setTestOpts(js_skip) test('space_leak_001', - # This could potentially be replaced with - # collect_stats('all',5) to test all 3 with - # 5% possible deviation. [ collect_stats('bytes allocated',5), collect_runtime_residency(15), omit_ways(['profasm','profthreaded','threaded1','threaded2', View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/b747e20aeee4f4fc7d08bc25b4d8682... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/b747e20aeee4f4fc7d08bc25b4d8682... You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications: https://gitlab.haskell.org/-/profile/notifications | Help: https://gitlab.haskell.org/help
participants (1)
-
Simon Jakobi (@sjakobi)