diff options
author | Matthew Pickering <matthewtpickering@gmail.com> | 2021-04-26 12:10:34 +0100 |
---|---|---|
committer | Marge Bot <ben+marge-bot@smart-cactus.org> | 2021-05-05 05:43:49 -0400 |
commit | e9617fbace826b048208c51e47d1552449675d57 (patch) | |
tree | a66e5a46e214ccd6c7505f138122840d6b1491da /testsuite | |
parent | 101d25fce311237b82580745c321f70e62755719 (diff) | |
download | haskell-e9617fbace826b048208c51e47d1552449675d57.tar.gz |
test driver: Make sure RESIDENCY_OPTS is passed for 'all' perf tests
Fixes #19731
-------------------------
Metric Decrease:
T11545
Metric Increase:
T12545
T15304
-------------------------
Diffstat (limited to 'testsuite')
-rw-r--r-- | testsuite/driver/testlib.py | 39 | ||||
-rw-r--r-- | testsuite/driver/testutil.py | 6 | ||||
-rw-r--r-- | testsuite/tests/perf/space_leaks/all.T | 2 |
3 files changed, 34 insertions, 13 deletions
diff --git a/testsuite/driver/testlib.py b/testsuite/driver/testlib.py index e8cb264ab5..f0a52bdefb 100644 --- a/testsuite/driver/testlib.py +++ b/testsuite/driver/testlib.py @@ -21,7 +21,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, \ + failBecause, testing_metrics, residency_testing_metrics, \ PassFail, badResult, memoize from term_color import Color, colored import testutil @@ -442,7 +442,7 @@ def _extra_files(name, opts, files): # 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 list of the metrics 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. @@ -472,7 +472,7 @@ def _collect_stats(name: TestName, opts, metrics, deviation, is_compiler_stats_t if metrics == 'all': metrics = testing_metrics() else: - metrics = [metrics] + metrics = { metrics } opts.is_stats_test = True if is_compiler_stats_test: @@ -486,6 +486,16 @@ def _collect_stats(name: TestName, opts, metrics, deviation, is_compiler_stats_t if config.compiler_debugged and is_compiler_stats_test: opts.skip = True + # If there are any residency testing metrics then turn on RESIDENCY_OPTS and + # omit nonmoving GC ways, which don't support profiling. + if residency_testing_metrics() & metrics: + if is_compiler_stats_test: + _extra_hc_opts(name, opts, RESIDENCY_OPTS) + else: + _extra_run_opts(name, opts, RESIDENCY_OPTS) + # The nonmoving collector does not support -G1 + _omit_ways(name, opts, [WayName(name) for name in ['nonmoving', 'nonmoving_thr', 'nonmoving_thr_ghc']]) + for metric_name in metrics: metric = '{}/{}'.format(tag, metric_name) def baselineByWay(way, target_commit, metric=metric): @@ -619,25 +629,32 @@ def have_thread_sanitizer( ) -> bool: # libraries to get a heap profile). # * view the heap profiles, read off the maximum residency. If it has # really changed, then you know there's an issue. +# +# As an example how much of a difference passing these flags make, here's +# an excerpt from CI metrics from the i386 job of !5604. Note that patch +# only passes RESIDENCY_OPTS to a couple more tests, which to be greatly +# affected: +# +# Test Metric value New value Change +# ---------------------------------------------------------------- +# T11545(normal) ghc/peak 68.2 56.0 -17.9% GOOD +# T15304(normal) ghc/peak 40.0 42.0 +5.0% +# T15630(normal) ghc/peak 16.0 15.0 -6.2% +# T15304(normal) ghc/max 14177634.7 16174668.0 +14.1% BAD +# T15630(normal) ghc/max 4583039.1 5457248.0 +19.1% RESIDENCY_OPTS = '+RTS -A256k -i0 -hT -RTS' # See Note [Measuring residency]. def collect_runtime_residency(tolerance_pct: float): return [ - collect_stats(['peak_megabytes_allocated', 'max_bytes_used'], tolerance_pct), - extra_run_opts(RESIDENCY_OPTS), - # The nonmoving collector does not support -G1 - omit_ways([WayName(name) for name in ['nonmoving', 'nonmoving_thr', 'nonmoving_thr_ghc']]) + collect_stats(residency_testing_metrics(), tolerance_pct), ] # See Note [Measuring residency]. def collect_compiler_residency(tolerance_pct: float): return [ - collect_compiler_stats(['peak_megabytes_allocated', 'max_bytes_used'], tolerance_pct), - extra_hc_opts(RESIDENCY_OPTS), - # The nonmoving collector does not support -G1 - omit_ways([WayName('nonmoving_thr_ghc')]) + collect_compiler_stats(residency_testing_metrics(), tolerance_pct) ] # --- diff --git a/testsuite/driver/testutil.py b/testsuite/driver/testutil.py index 064fd8086f..bcea98bd17 100644 --- a/testsuite/driver/testutil.py +++ b/testsuite/driver/testutil.py @@ -78,7 +78,11 @@ def lndir(srcdir: Path, dstdir: Path): # All possible test metric strings. def testing_metrics(): - return ['bytes allocated', 'peak_megabytes_allocated', 'max_bytes_used'] + return { 'bytes allocated', 'peak_megabytes_allocated', 'max_bytes_used' } + +# Metrics which are testing residency information +def residency_testing_metrics(): + return { 'peak_megabytes_allocated', 'max_bytes_used' } # On Windows, os.symlink is not defined with Python 2.7, but is in Python 3 # when using msys2, as GHC does. Unfortunately, only Administrative users have diff --git a/testsuite/tests/perf/space_leaks/all.T b/testsuite/tests/perf/space_leaks/all.T index 6c54f46e3b..deb5ea38e4 100644 --- a/testsuite/tests/perf/space_leaks/all.T +++ b/testsuite/tests/perf/space_leaks/all.T @@ -3,7 +3,7 @@ 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_stats('bytes allocated',5), collect_runtime_residency(15), omit_ways(['profasm','profthreaded','threaded1','threaded2','nonmoving_thr']) ], |