Simon Jakobi pushed to branch wip/sjakobi/T27602 at Glasgow Haskell Compiler / GHC Commits: 736fcdfb by Simon Jakobi at 2026-08-17T10:25:36+02:00 testsuite: Show baseline sample count and range in perf failures A perf baseline is the mean of all samples recorded for a commit, and it prints as a single number, hiding how far the samples spread. When the spread is wide, this can indicate an unstable metric that isn't actually useful as a signal for the perf tests. For example, in #27602, T27336's peak_megabytes_allocated baseline showed as 757 when the underlying samples were 605 and 909. When the baseline is averaged from more than one sample, say so in the failure output: the one-line stat-failure reason shows the sample range, and the detail block lists the raw samples. Single-sample baselines print exactly as before. Context: #27602 Assisted-by: Claude Fable 5 - - - - - 29b1d269 by Simon Jakobi at 2026-08-17T10:30:11+02:00 ci: Clarify comment on pushing perf notes after failures Context: #27602 Assisted-by: Claude Fable 5 - - - - - 9fc0bcaf by Simon Jakobi at 2026-08-17T10:30:11+02:00 DEMO: Tighten T3064 residency tolerance (do not merge) Assisted-by: Claude Fable 5 - - - - - 3 changed files: - .gitlab/ci.sh - testsuite/driver/perf_notes.py - testsuite/tests/perf/compiler/all.T Changes: ===================================== .gitlab/ci.sh ===================================== @@ -1120,9 +1120,10 @@ case ${1:-help} in setup) setup && cleanup_submodules ;; configure) time_it "configure" configure ;; build_hadrian) time_it "build" build_hadrian ;; - # N.B. Always push notes, even if the build fails. This is okay to do as the - # testsuite driver doesn't record notes for tests that fail due to - # correctness. + # N.B. Always push notes, even if the build fails. Metrics from runs failing + # a perf stat check are deliberately recorded too — discarding them would + # bias the baseline towards whichever sample came first. Only correctness + # failures record nothing. test_hadrian) fetch_perf_notes res=0 ===================================== testsuite/driver/perf_notes.py ===================================== @@ -83,9 +83,18 @@ PerfStat = NamedTuple('PerfStat', [('test_env', TestEnv), ('metric', MetricName), ('value', float)]) +# A test's metric recovered from a commit's git note: the raw sample values +# recorded there, and a PerfStat whose value is their mean. +class CommitMetric(NamedTuple): + perfStat: PerfStat + samples: List[float] + # A baseline recovered form stored metrics. -Baseline = NamedTuple('Baseline', [('perfStat', PerfStat), - ('commit', GitHash)]) +class Baseline(NamedTuple): + perfStat: PerfStat + commit: GitHash + # The raw samples the baseline value was averaged over. + samples: List[float] = [] # The type of exceptions which are thrown when computing the current stat value # fails. @@ -460,10 +469,10 @@ def get_allowed_changes(baseline_ref: Optional[GitRef]) -> Dict[TestName, List[A else: return get_allowed_perf_changes() -# Cache of baseline values. This is a dict of dicts indexed on: -# (useCiNamespace, commit) -> (test_env, test, metric, way) -> baseline -# (bool , str ) -> (str , str , str , str) -> float -_commit_metric_cache = {} # type: ignore +# Cache of commit metrics. +_commit_metric_cache: Dict[Tuple[NoteNamespace, GitHash], + Dict[Tuple[TestEnv, TestName, MetricName, WayName], + CommitMetric]] = {} # Get the baseline of a test at a given commit. This is the expected value # *before* the commit is applied (i.e. on the parent commit). @@ -506,7 +515,8 @@ def baseline_metric(commit: GitHash, if baseline_commit is not None: current_metric = get_commit_metric(namespace, baseline_commit, test_env, name, metric, way) if current_metric is not None: - return Baseline(current_metric, baseline_commit) + return Baseline(current_metric.perfStat, baseline_commit, + current_metric.samples) else: return None @@ -515,7 +525,8 @@ def baseline_metric(commit: GitHash, # Check for a metric on this commit. current_metric = get_commit_metric(namespace, current_commit, test_env, name, metric, way) if current_metric is not None: - return Baseline(current_metric, current_commit) + return Baseline(current_metric.perfStat, current_commit, + current_metric.samples) # Stop if there is an expected change at this commit. In that case # metrics on ancestor commits will not be a valid baseline. @@ -545,23 +556,23 @@ def get_commit_metric_value_str_or_none(gitNoteRef, result = get_commit_metric(gitNoteRef, commit, test_env, name, metric, way) if result is None: return None - return str(result.value) + return str(result.perfStat.value) -# gets the average commit metric from git notes. +# gets the commit metric (average and raw samples) from git notes. # gitNoteRef: git notes ref space e.g. "perf" or "ci/perf" # ref: git commit # test_env: test environment # name: test name # metric: test metric # way: test way -# returns: PerfStat | None if stats don't exist for the given input +# returns: CommitMetric | None if stats don't exist for the given input def get_commit_metric(gitNoteRef, ref: Union[GitRef, GitHash], test_env: TestEnv, name: TestName, metric: MetricName, way: WayName - ) -> Optional[PerfStat]: + ) -> Optional[CommitMetric]: global _commit_metric_cache assert test_env != None commit = commit_hash(ref) @@ -573,9 +584,9 @@ def get_commit_metric(gitNoteRef, return _commit_metric_cache[cacheKeyA].get(cacheKeyB) # Cache miss. - # Calculate baselines from the current commit's git note. + # Calculate metrics from the current commit's git note. # Note that the git note may contain data for other tests. All tests' - # baselines will be collected and cached for future use. + # metrics will be collected and cached for future use. allCommitMetrics = get_perf_stats(ref, gitNoteRef) # Collect recorded values by cacheKeyB. @@ -586,19 +597,28 @@ def get_commit_metric(gitNoteRef, currentValues = values_by_cache_key_b.setdefault(currentCacheKey, []) currentValues.append(float(perfStat.value)) - # Calculate and baseline (average of values) by cacheKeyB. - baseline_by_cache_key_b = {} + # Calculate the metric (average of values, plus the values themselves) + # by cacheKeyB. + metric_by_cache_key_b = {} for currentCacheKey, currentValues in values_by_cache_key_b.items(): - baseline_by_cache_key_b[currentCacheKey] = PerfStat( \ - currentCacheKey[0], - currentCacheKey[1], - currentCacheKey[3], - currentCacheKey[2], - sum(currentValues) / len(currentValues)) + metric_by_cache_key_b[currentCacheKey] = CommitMetric( + PerfStat( + currentCacheKey[0], + currentCacheKey[1], + currentCacheKey[3], + currentCacheKey[2], + sum(currentValues) / len(currentValues)), + currentValues) + + # Save metrics to the cache. + _commit_metric_cache[cacheKeyA] = metric_by_cache_key_b + return metric_by_cache_key_b.get(cacheKeyB) - # Save baselines to the cache. - _commit_metric_cache[cacheKeyA] = baseline_by_cache_key_b - return baseline_by_cache_key_b.get(cacheKeyB) +def format_sample(s: float) -> str: + return str(int(s)) if s == int(s) else str(s) + +def format_samples(samples: List[float]) -> str: + return ', '.join(format_sample(s) for s in samples) def check_stats_change(actual: PerfStat, baseline: Baseline, @@ -654,9 +674,17 @@ def check_stats_change(actual: PerfStat, ' baseline @ %s' % baseline.commit print(actual.metric, error + ':') dev = 100.0 if expected_val == 0 else round(((float(actual.value) * 100) / int(expected_val)) - 100, 1) + # Show the sample spread so unreliable baselines become visible (#27602). + if len(baseline.samples) > 1: + samples_note = ('; baseline is mean of %d samples spanning %s..%s' + % (len(baseline.samples), + format_sample(min(baseline.samples)), + format_sample(max(baseline.samples)))) + else: + samples_note = '' change_line = (f'{actual.metric} {change.value} from {baseline.perfStat.test_env} ' f'baseline @ {baseline.commit[:7]}: {expected_val} -> {actual.value} ' - f'({dev:+g}%, allowed {acceptance_window.describe()})') + f'({dev:+g}%, allowed {acceptance_window.describe()}{samples_note})') result = failBecause('stat ' + change_line, tag='stat') if not change_allowed or force_print: @@ -666,6 +694,10 @@ def check_stats_change(actual: PerfStat, print(descr, str(val).rjust(length), extra) display(' Expected ' + full_name + ' ' + actual.metric + ':', expected_val, acceptance_window.describe()) + if len(baseline.samples) > 1: + display(' Samples ' + full_name + ' ' + actual.metric + ':', + len(baseline.samples), + '(' + format_samples(baseline.samples) + ')') display(' Lower bound ' + full_name + ' ' + actual.metric + ':', lowerBound, '') display(' Upper bound ' + full_name + ' ' + actual.metric + ':', upperBound, '') display(' Actual ' + full_name + ' ' + actual.metric + ':', actual.value, '') @@ -866,7 +898,7 @@ def main() -> None: # HEAD~2 21234 21234 # HEAD~3 20000 20000 def strMetric(x): - return '{:.2f}'.format(x.value) if x != None else "" + return '{:.2f}'.format(x.perfStat.value) if x != None else "" # Data is in column major format, so transpose and pass to print_table. T = TypeVar('T') def transpose(xss: List[List[T]]) -> List[List[T]]: ===================================== testsuite/tests/perf/compiler/all.T ===================================== @@ -48,7 +48,9 @@ test('T4801', ['']) test('T3064', - [collect_compiler_residency(20), + [# Demo-only: tightened from 20 so the bimodal max_bytes_used + # baseline on aarch64-linux-deb13-validate fails. + collect_compiler_residency(1), collect_compiler_runtime(2), only_ways(['normal']), ], View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/324435b2d2df03e43ffbb8059a28509... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/324435b2d2df03e43ffbb8059a28509... 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)