[Git][ghc/ghc][wip/sjakobi/T27602] 3 commits: testsuite: Show baseline sample count and spread in perf failures
Simon Jakobi pushed to branch wip/sjakobi/T27602 at Glasgow Haskell Compiler / GHC Commits: c9aaa39c by Simon Jakobi at 2026-08-16T21:15:10+02:00 testsuite: Show baseline sample count and spread in perf failures A perf baseline is the mean of all samples recorded for a commit, so a single outlier can silently corrupt it. Previously, the failure output gave no hint about such outliers: the baseline printed as one number. 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 message and list the samples, both in the one-line stat-failure reason and in the detail block. Single-sample baselines print exactly as before. Context: #27602 Assisted-by: Claude Fable 5 - - - - - 2ad08df6 by Simon Jakobi at 2026-08-16T21:15:10+02:00 ci: Clarify comment on pushing perf notes after failures Context: #27602 Assisted-by: Claude Fable 5 - - - - - 89a9867d by Simon Jakobi at 2026-08-16T21:15:10+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,9 +469,8 @@ 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 +# Cache of commit metrics. This is a dict of dicts indexed on: +# (useCiNamespace, commit) -> (test_env, test, metric, way) -> CommitMetric _commit_metric_cache = {} # type: ignore # Get the baseline of a test at a given commit. This is the expected value @@ -506,7 +514,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 +524,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 +555,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 +583,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 +596,25 @@ 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)) - - # Save baselines to the cache. - _commit_metric_cache[cacheKeyA] = baseline_by_cache_key_b - return baseline_by_cache_key_b.get(cacheKeyB) + 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) + +def format_samples(samples: List[float]) -> str: + return ', '.join(str(int(s)) if s == int(s) else str(s) for s in samples) def check_stats_change(actual: PerfStat, baseline: Baseline, @@ -654,9 +670,16 @@ 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 samples so outliers become visible (#27602). + if len(baseline.samples) > 1: + samples_note = ('; baseline is mean of %d samples: %s' + % (len(baseline.samples), + format_samples(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 +689,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 +893,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/e17c0f57f0a408b8c715a50366d7b8d... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/e17c0f57f0a408b8c715a50366d7b8d... 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)