Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC

Commits:

3 changed files:

Changes:

  • .gitlab/ci.sh
    ... ... @@ -1120,9 +1120,10 @@ case ${1:-help} in
    1120 1120
       setup) setup && cleanup_submodules ;;
    
    1121 1121
       configure) time_it "configure" configure ;;
    
    1122 1122
       build_hadrian) time_it "build" build_hadrian ;;
    
    1123
    -  # N.B. Always push notes, even if the build fails. This is okay to do as the
    
    1124
    -  # testsuite driver doesn't record notes for tests that fail due to
    
    1125
    -  # correctness.
    
    1123
    +  # N.B. Always push notes, even if the build fails. Metrics from runs failing
    
    1124
    +  # a perf stat check are deliberately recorded too — discarding them would
    
    1125
    +  # bias the baseline towards whichever sample came first. Only correctness
    
    1126
    +  # failures record nothing.
    
    1126 1127
       test_hadrian)
    
    1127 1128
         fetch_perf_notes
    
    1128 1129
         res=0
    

  • testsuite/driver/perf_notes.py
    ... ... @@ -83,9 +83,13 @@ PerfStat = NamedTuple('PerfStat', [('test_env', TestEnv),
    83 83
                                        ('metric', MetricName),
    
    84 84
                                        ('value', float)])
    
    85 85
     
    
    86
    -# A baseline recovered form stored metrics.
    
    87
    -Baseline = NamedTuple('Baseline', [('perfStat', PerfStat),
    
    88
    -                                   ('commit', GitHash)])
    
    86
    +# A test's metric recovered from a commit's git note: the raw sample values
    
    87
    +# recorded there, and a PerfStat whose value is their mean. Serves as the
    
    88
    +# baseline when comparing a test run against an earlier commit.
    
    89
    +class CommitMetric(NamedTuple):
    
    90
    +    perfStat: PerfStat
    
    91
    +    commit: GitHash
    
    92
    +    samples: List[float]
    
    89 93
     
    
    90 94
     # The type of exceptions which are thrown when computing the current stat value
    
    91 95
     # fails.
    
    ... ... @@ -460,10 +464,10 @@ def get_allowed_changes(baseline_ref: Optional[GitRef]) -> Dict[TestName, List[A
    460 464
      else:
    
    461 465
             return get_allowed_perf_changes()
    
    462 466
     
    
    463
    -# Cache of baseline values. This is a dict of dicts indexed on:
    
    464
    -# (useCiNamespace, commit) -> (test_env, test, metric, way) -> baseline
    
    465
    -# (bool          , str   ) -> (str     , str , str   , str) -> float
    
    466
    -_commit_metric_cache = {} # type: ignore
    
    467
    +# Cache of commit metrics.
    
    468
    +_commit_metric_cache: Dict[Tuple[NoteNamespace, GitHash],
    
    469
    +                           Dict[Tuple[TestEnv, TestName, MetricName, WayName],
    
    470
    +                                CommitMetric]] = {}
    
    467 471
     
    
    468 472
     # Get the baseline of a test at a given commit. This is the expected value
    
    469 473
     # *before* the commit is applied (i.e. on the parent commit).
    
    ... ... @@ -477,7 +481,7 @@ _commit_metric_cache = {} # type: ignore
    477 481
     #                      instead when looking for ci results)
    
    478 482
     # metric: str - test metric
    
    479 483
     # way: str - test way
    
    480
    -# returns: the Baseline or None if no metric was found within
    
    484
    +# returns: the baseline CommitMetric or None if no metric was found within
    
    481 485
     #          BaselineSearchDepth commits and since the last expected change
    
    482 486
     #          (ignoring any expected change in the given commit).
    
    483 487
     def baseline_metric(commit: GitHash,
    
    ... ... @@ -486,7 +490,7 @@ def baseline_metric(commit: GitHash,
    486 490
                         metric: MetricName,
    
    487 491
                         way: WayName,
    
    488 492
                         baseline_ref: Optional[GitRef]
    
    489
    -                    ) -> Optional[Baseline]:
    
    493
    +                    ) -> Optional[CommitMetric]:
    
    490 494
         # For performance reasons (in order to avoid calling commit_hash), we assert
    
    491 495
         # commit is already a commit hash.
    
    492 496
         assert is_commit_hash(commit)
    
    ... ... @@ -502,20 +506,16 @@ def baseline_metric(commit: GitHash,
    502 506
         # Searches through previous commits trying local then ci for each commit in.
    
    503 507
         def find_baseline(namespace: NoteNamespace,
    
    504 508
                           test_env: TestEnv
    
    505
    -                      ) -> Optional[Baseline]:
    
    509
    +                      ) -> Optional[CommitMetric]:
    
    506 510
             if baseline_commit is not None:
    
    507
    -            current_metric = get_commit_metric(namespace, baseline_commit, test_env, name, metric, way)
    
    508
    -            if current_metric is not None:
    
    509
    -                return Baseline(current_metric, baseline_commit)
    
    510
    -            else:
    
    511
    -                return None
    
    511
    +            return get_commit_metric(namespace, baseline_commit, test_env, name, metric, way)
    
    512 512
     
    
    513 513
             for depth, current_commit in list(enumerate(commit_hashes)):
    
    514 514
                 if current_commit == commit: continue
    
    515 515
                 # Check for a metric on this commit.
    
    516 516
                 current_metric = get_commit_metric(namespace, current_commit, test_env, name, metric, way)
    
    517 517
                 if current_metric is not None:
    
    518
    -                return Baseline(current_metric, current_commit)
    
    518
    +                return current_metric
    
    519 519
     
    
    520 520
                 # Stop if there is an expected change at this commit. In that case
    
    521 521
                 # metrics on ancestor commits will not be a valid baseline.
    
    ... ... @@ -527,7 +527,7 @@ def baseline_metric(commit: GitHash,
    527 527
         # Test environment to use when comparing against CI namespace
    
    528 528
         ci_test_env = best_fit_ci_test_env()
    
    529 529
     
    
    530
    -    baseline = find_baseline(LocalNamespace, test_env) # type: Optional[Baseline]
    
    530
    +    baseline = find_baseline(LocalNamespace, test_env) # type: Optional[CommitMetric]
    
    531 531
         if baseline is None and ci_test_env is not None:
    
    532 532
             baseline = find_baseline(CiNamespace, ci_test_env)
    
    533 533
     
    
    ... ... @@ -545,23 +545,23 @@ def get_commit_metric_value_str_or_none(gitNoteRef,
    545 545
         result = get_commit_metric(gitNoteRef, commit, test_env, name, metric, way)
    
    546 546
         if result is None:
    
    547 547
             return None
    
    548
    -    return str(result.value)
    
    548
    +    return str(result.perfStat.value)
    
    549 549
     
    
    550
    -# gets the average commit metric from git notes.
    
    550
    +# gets the commit metric (average and raw samples) from git notes.
    
    551 551
     # gitNoteRef: git notes ref space e.g. "perf" or "ci/perf"
    
    552 552
     # ref: git commit
    
    553 553
     # test_env: test environment
    
    554 554
     # name: test name
    
    555 555
     # metric: test metric
    
    556 556
     # way: test way
    
    557
    -# returns: PerfStat | None if stats don't exist for the given input
    
    557
    +# returns: CommitMetric | None if stats don't exist for the given input
    
    558 558
     def get_commit_metric(gitNoteRef,
    
    559 559
                           ref: Union[GitRef, GitHash],
    
    560 560
                           test_env: TestEnv,
    
    561 561
                           name: TestName,
    
    562 562
                           metric: MetricName,
    
    563 563
                           way: WayName
    
    564
    -                      ) -> Optional[PerfStat]:
    
    564
    +                      ) -> Optional[CommitMetric]:
    
    565 565
         global _commit_metric_cache
    
    566 566
         assert test_env != None
    
    567 567
         commit = commit_hash(ref)
    
    ... ... @@ -573,9 +573,9 @@ def get_commit_metric(gitNoteRef,
    573 573
             return _commit_metric_cache[cacheKeyA].get(cacheKeyB)
    
    574 574
     
    
    575 575
         # Cache miss.
    
    576
    -    # Calculate baselines from the current commit's git note.
    
    576
    +    # Calculate metrics from the current commit's git note.
    
    577 577
         # Note that the git note may contain data for other tests. All tests'
    
    578
    -    # baselines will be collected and cached for future use.
    
    578
    +    # metrics will be collected and cached for future use.
    
    579 579
         allCommitMetrics = get_perf_stats(ref, gitNoteRef)
    
    580 580
     
    
    581 581
         # Collect recorded values by cacheKeyB.
    
    ... ... @@ -586,22 +586,32 @@ def get_commit_metric(gitNoteRef,
    586 586
             currentValues = values_by_cache_key_b.setdefault(currentCacheKey, [])
    
    587 587
             currentValues.append(float(perfStat.value))
    
    588 588
     
    
    589
    -    # Calculate and baseline (average of values) by cacheKeyB.
    
    590
    -    baseline_by_cache_key_b = {}
    
    589
    +    # Calculate the metric (average of values, plus the values themselves)
    
    590
    +    # by cacheKeyB.
    
    591
    +    metric_by_cache_key_b = {}
    
    591 592
         for currentCacheKey, currentValues in values_by_cache_key_b.items():
    
    592
    -        baseline_by_cache_key_b[currentCacheKey] = PerfStat( \
    
    593
    -                currentCacheKey[0],
    
    594
    -                currentCacheKey[1],
    
    595
    -                currentCacheKey[3],
    
    596
    -                currentCacheKey[2],
    
    597
    -                sum(currentValues) / len(currentValues))
    
    598
    -
    
    599
    -    # Save baselines to the cache.
    
    600
    -    _commit_metric_cache[cacheKeyA] = baseline_by_cache_key_b
    
    601
    -    return baseline_by_cache_key_b.get(cacheKeyB)
    
    593
    +        metric_by_cache_key_b[currentCacheKey] = CommitMetric(
    
    594
    +                PerfStat(
    
    595
    +                    currentCacheKey[0],
    
    596
    +                    currentCacheKey[1],
    
    597
    +                    currentCacheKey[3],
    
    598
    +                    currentCacheKey[2],
    
    599
    +                    sum(currentValues) / len(currentValues)),
    
    600
    +                commit,
    
    601
    +                currentValues)
    
    602
    +
    
    603
    +    # Save metrics to the cache.
    
    604
    +    _commit_metric_cache[cacheKeyA] = metric_by_cache_key_b
    
    605
    +    return metric_by_cache_key_b.get(cacheKeyB)
    
    606
    +
    
    607
    +def format_sample(s: float) -> str:
    
    608
    +    return str(int(s)) if s == int(s) else str(s)
    
    609
    +
    
    610
    +def format_samples(samples: List[float]) -> str:
    
    611
    +    return ', '.join(format_sample(s) for s in samples)
    
    602 612
     
    
    603 613
     def check_stats_change(actual: PerfStat,
    
    604
    -                       baseline: Baseline,
    
    614
    +                       baseline: CommitMetric,
    
    605 615
                            acceptance_window: MetricAcceptanceWindow,
    
    606 616
                            allowed_perf_changes: Dict[TestName, List[AllowedPerfChange]] = {},
    
    607 617
                            force_print = False
    
    ... ... @@ -611,8 +621,8 @@ def check_stats_change(actual: PerfStat,
    611 621
     
    
    612 622
         Parameters:
    
    613 623
         actual: the PerfStat with actual value
    
    614
    -    baseline: the expected Baseline value (this should generally be derived
    
    615
    -        from baseline_metric())
    
    624
    +    baseline: the CommitMetric to compare against (this should generally be
    
    625
    +        derived from baseline_metric())
    
    616 626
         acceptance_window: allowed deviation of the actual value from the expected
    
    617 627
             value.
    
    618 628
         allowed_perf_changes: allowed changes in stats. This is a dictionary as
    
    ... ... @@ -654,9 +664,17 @@ def check_stats_change(actual: PerfStat,
    654 664
                     ' baseline @ %s' % baseline.commit
    
    655 665
             print(actual.metric, error + ':')
    
    656 666
             dev = 100.0 if expected_val == 0 else round(((float(actual.value) * 100) / int(expected_val)) - 100, 1)
    
    667
    +        # Show the sample spread so unreliable baselines become visible (#27602).
    
    668
    +        if len(baseline.samples) > 1:
    
    669
    +            samples_note = ('; baseline is mean of %d samples spanning %s..%s'
    
    670
    +                            % (len(baseline.samples),
    
    671
    +                               format_sample(min(baseline.samples)),
    
    672
    +                               format_sample(max(baseline.samples))))
    
    673
    +        else:
    
    674
    +            samples_note = ''
    
    657 675
             change_line = (f'{actual.metric} {change.value} from {baseline.perfStat.test_env} '
    
    658 676
                            f'baseline @ {baseline.commit[:7]}: {expected_val} -> {actual.value} '
    
    659
    -                       f'({dev:+g}%, allowed {acceptance_window.describe()})')
    
    677
    +                       f'({dev:+g}%, allowed {acceptance_window.describe()}{samples_note})')
    
    660 678
             result = failBecause('stat ' + change_line, tag='stat')
    
    661 679
     
    
    662 680
         if not change_allowed or force_print:
    
    ... ... @@ -666,6 +684,10 @@ def check_stats_change(actual: PerfStat,
    666 684
                 print(descr, str(val).rjust(length), extra)
    
    667 685
     
    
    668 686
             display('    Expected    ' + full_name + ' ' + actual.metric + ':', expected_val, acceptance_window.describe())
    
    687
    +        if len(baseline.samples) > 1:
    
    688
    +            display('    Samples     ' + full_name + ' ' + actual.metric + ':',
    
    689
    +                    len(baseline.samples),
    
    690
    +                    '(' + format_samples(baseline.samples) + ')')
    
    669 691
             display('    Lower bound ' + full_name + ' ' + actual.metric + ':', lowerBound, '')
    
    670 692
             display('    Upper bound ' + full_name + ' ' + actual.metric + ':', upperBound, '')
    
    671 693
             display('    Actual      ' + full_name + ' ' + actual.metric + ':', actual.value, '')
    
    ... ... @@ -866,7 +888,7 @@ def main() -> None:
    866 888
         # HEAD~2           21234                 21234
    
    867 889
         # HEAD~3           20000                 20000
    
    868 890
         def strMetric(x):
    
    869
    -        return '{:.2f}'.format(x.value) if x != None else ""
    
    891
    +        return '{:.2f}'.format(x.perfStat.value) if x != None else ""
    
    870 892
         # Data is in column major format, so transpose and pass to print_table.
    
    871 893
         T = TypeVar('T')
    
    872 894
         def transpose(xss: List[List[T]]) -> List[List[T]]:
    

  • testsuite/driver/testglobals.py
    ... ... @@ -4,7 +4,7 @@
    4 4
     
    
    5 5
     from my_typing import *
    
    6 6
     from pathlib import Path
    
    7
    -from perf_notes import MetricChange, PerfStat, Baseline, GitRef
    
    7
    +from perf_notes import MetricChange, PerfStat, CommitMetric, GitRef
    
    8 8
     from datetime import datetime
    
    9 9
     
    
    10 10
     # -----------------------------------------------------------------------------
    
    ... ... @@ -312,7 +312,7 @@ class TestResult:
    312 312
     PerfMetric = NamedTuple('PerfMetric',
    
    313 313
                             [('change', MetricChange),
    
    314 314
                              ('stat', PerfStat),
    
    315
    -                         ('baseline', Optional[Baseline]) ])
    
    315
    +                         ('baseline', Optional[CommitMetric]) ])
    
    316 316
     
    
    317 317
     class TestRun:
    
    318 318
        def __init__(self) -> None: