Split collect_stats into collect_stats and collect_compiler_statswip/perf-testsuite

6 files changed, 70 insertions, 59 deletions

diff --git a/testsuite/driver/README.md b/testsuite/driver/README.md
index 6ae7db6842..786f98a6f5 100644
--- a/testsuite/driver/README.md
+++ b/testsuite/driver/README.md
@@ -34,11 +34,15 @@ 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',5)` to
+As such, it is perfectly sufficient to write `collect_stats('all',20)` 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 5%
-change in any direction.  In fact, even that is not necessary as
-`collect_stats()` defaults to 'all', and 20% deviation allowed.  See the
+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.
+
+The function `collect_compiler_stats()` is completely equivalent in every way to
+`collect_stats` except that it measures the performance of the compiler itself
+rather than the performance of the code generated by the compiler. See the
 implementation of collect_stats in /driver/perf_notes.py for more information.
 
 If the performance of a test is improved so much that the test fails, the value
@@ -138,7 +142,7 @@ you checkout that commit and run the testsuite (or test).
 * accumulate_metrics
     - Exists in testglobals.py
     - A list of strings in the form `'\t'.join('test_env','test','way',
-      'metric','value')` 
+      'metric','value')`
       This is what is written to the git notes for the HEAD commit.  As such,
       this 'value' /does/ represent the actual performance measured during the
       test-suite's run.
diff --git a/testsuite/driver/perf_notes.py b/testsuite/driver/perf_notes.py
index b94d4568a5..1e5f19a719 100644
--- a/testsuite/driver/perf_notes.py
+++ b/testsuite/driver/perf_notes.py
@@ -85,7 +85,7 @@ def test_cmp(full_name, field, val, expected, dev=20):
 # Corresponds to 'all' setting for metric parameter in collect_stats function.
 testing_metrics = ['bytes allocated', 'peak_megabytes_allocated', 'max_bytes_used']
 
-# Defaults to "test everything, only break on extreme cases, not a compiler stats test"
+# Defaults to "test everything, and only break on extreme cases"
 #
 # The inputs to this function are slightly interesting:
 # metric can be either:
@@ -93,19 +93,26 @@ testing_metrics = ['bytes allocated', 'peak_megabytes_allocated', 'max_bytes_use
 #     - The specific metric one wants to use in the test.
 #     - A list of the metrics one wants to use in the test.
 #
-# deviation defaults to 20% because the goal is correctness over performance.
+# 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.
 #
-# 'compiler' is somewhat of an unfortunate name.
+# 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):
+    return lambda name, opts, m=metric, d=deviation: _collect_stats(name, opts, m,d, True)
+
+def collect_stats(metric='all', deviation=20):
+    return lambda name, opts, m=metric, d=deviation: _collect_stats(name, opts, m, d)
+
+# 'is_compiler_stats_test' is somewhat of an unfortunate name.
 # If the boolean is set to true, it indicates that this test is one that
 # measures the performance numbers of the compiler.
 # As this is a fairly rare case in the testsuite, it defaults to false to
 # indicate that it is a 'normal' performance test.
-def collect_stats(metric='all', deviation=20, compiler=False):
-    return lambda name, opts, m=metric, d=deviation, c=compiler: _collect_stats(name, opts, m, d, c)
-
-def _collect_stats(name, opts, metric, deviation, is_compiler_stats_test):
+# This is an internal function that is used only in the implementation.
+def _collect_stats(name, opts, metric, deviation, is_compiler_stats_test=False):
     if not re.match('^[0-9]*[a-zA-Z][a-zA-Z0-9._-]*$', name):
         # my_framework_fail(name, 'bad_name', 'This test has an invalid name')
         my_failBecause('This test has an invalid name.')
diff --git a/testsuite/driver/testlib.py b/testsuite/driver/testlib.py
index 660ce37657..44f1ded0a7 100644
--- a/testsuite/driver/testlib.py
+++ b/testsuite/driver/testlib.py
@@ -23,7 +23,7 @@ import subprocess
 
 from testglobals import *
 from testutil import *
-from perf_notes import collect_stats, evaluate_metric
+from perf_notes import collect_compiler_stats, collect_stats, evaluate_metric
 extra_src_files = {'T4198': ['exitminus1.c']} # TODO: See #12223
 
 if config.use_threads:
diff --git a/testsuite/tests/deriving/perf/all.T b/testsuite/tests/deriving/perf/all.T
index 46b980f1bd..1402a38b5d 100644
--- a/testsuite/tests/deriving/perf/all.T
+++ b/testsuite/tests/deriving/perf/all.T
@@ -1,5 +1,5 @@
 test('T10858',
-     [ collect_stats('bytes allocated',8,True),
+     [ collect_compiler_stats('bytes allocated',8),
       only_ways(['normal'])],
      compile,
      ['-O'])
diff --git a/testsuite/tests/perf/compiler/all.T b/testsuite/tests/perf/compiler/all.T
index 7db144236d..9d66716ef1 100644
--- a/testsuite/tests/perf/compiler/all.T
+++ b/testsuite/tests/perf/compiler/all.T
@@ -1,4 +1,4 @@
-# Tests that call 'collect_stats(_,_,True)' are tests that measure compiler
+# Tests that call 'collect_compiler_stats(_,_)' are tests that measure compiler
 # performance statistics and are skipped when debugging is on.
 # See testsuite/driver/testlib.py.
 
@@ -30,8 +30,8 @@ setTestOpts(no_lint)
 
 test('T1969',
      [# expect_broken(12437),
-      collect_stats(['peak_megabytes_allocated','max_bytes_used'],15,True),
-      collect_stats('bytes allocated',5,True),
+      collect_compiler_stats(['peak_megabytes_allocated','max_bytes_used'],15),
+      collect_compiler_stats('bytes allocated',5),
       only_ways(['normal']),
       extra_hc_opts('-dcore-lint -static'),
           # Leave -dcore-lint on for this one test, so that we have something
@@ -57,8 +57,8 @@ else:
 
 test('T3294',
      [
-      collect_stats('max_bytes_used',15,True),
-      collect_stats('bytes allocated',5,True),
+      collect_compiler_stats('max_bytes_used',15),
+      collect_compiler_stats('bytes allocated',5),
       conf_3294,
 
       # Use `+RTS -G1` for more stable residency measurements. Note [residency].
@@ -68,14 +68,14 @@ test('T3294',
      [''])
 
 test('T4801',
-     [ # collect_stats('peak_megabytes_allocated',1,True),
+     [ # collect_compiler_stats('peak_megabytes_allocated',1),
      # expect_broken(5224),
      # temporarily unbroken (#5227)
      # deactivated for now, as this metric became too volatile recently
 
-      collect_stats('bytes allocated',10,True),
+      collect_compiler_stats('bytes allocated',10),
 
-       # collect_stats('max_bytes_used',5,True),
+       # collect_compiler_stats('max_bytes_used',5),
        # deactivated for now, as this metric became too volatile recently
        only_ways(['normal']),
        extra_hc_opts('-static'),
@@ -87,11 +87,11 @@ test('T4801',
      [''])
 
 test('T3064',
-     [collect_stats('peak_megabytes_allocated',20,True),
+     [collect_compiler_stats('peak_megabytes_allocated',20),
 
-      collect_stats('bytes allocated',10,True),
+      collect_compiler_stats('bytes allocated',10),
 
-      # collect_stats('max_bytes_used',20,True),
+      # collect_compiler_stats('max_bytes_used',20),
       # deactivated for now, as this metric became too volatile recently
 
        only_ways(['normal']),
@@ -108,7 +108,7 @@ test('T4007',
      ['$MAKE -s --no-print-directory T4007'])
 
 test('T5030',
-     [collect_stats('bytes allocated',10,True),
+     [collect_compiler_stats('bytes allocated',10),
 
        only_ways(['normal'])
       ],
@@ -116,14 +116,14 @@ test('T5030',
      ['-freduction-depth=300'])
 
 test('T5631',
-     [collect_stats('bytes allocated',10,True),
+     [collect_compiler_stats('bytes allocated',10),
        only_ways(['normal'])
       ],
      compile,
      [''])
 
 test('parsing001',
-     [collect_stats('bytes allocated',10,True),
+     [collect_compiler_stats('bytes allocated',10),
        only_ways(['normal']),
       ],
      compile_fail, [''])
@@ -131,7 +131,7 @@ test('parsing001',
 
 test('T783',
      [ only_ways(['normal']),  # no optimisation for this one
-      collect_stats('bytes allocated',10,True),
+      collect_compiler_stats('bytes allocated',10),
 
       extra_hc_opts('-static')
       ],
@@ -139,44 +139,44 @@ test('T783',
 
 test('T5321Fun',
      [ only_ways(['normal']),  # no optimisation for this one
-       collect_stats('bytes allocated',10,True),
+       collect_compiler_stats('bytes allocated',10),
       ],
       compile,[''])
 
 test('T5321FD',
      [ only_ways(['normal']),  # no optimisation for this one
-      collect_stats('bytes allocated',10,True),
+      collect_compiler_stats('bytes allocated',10),
       ],
       compile,[''])
 
 test('T5642',
      [ only_ways(['normal']),
        normal,
-       collect_stats('bytes allocated',10,True),
+       collect_compiler_stats('bytes allocated',10),
       ],
       compile,['-O'])
 
 test('T5837',
      [ only_ways(['normal']),
-      collect_stats('bytes allocated',10,True),
+      collect_compiler_stats('bytes allocated',10),
       ],
       compile, ['-freduction-depth=50'])
 
 test('T6048',
      [ only_ways(['optasm']),
-     collect_stats('bytes allocated',10,True),
+     collect_compiler_stats('bytes allocated',10),
       ],
       compile,[''])
 
 test('T9020',
      [ only_ways(['optasm']),
-      collect_stats('bytes allocated',10,True),
+      collect_compiler_stats('bytes allocated',10),
       ],
       compile,[''])
 
 test('T9675',
      [ only_ways(['optasm']),
-       collect_stats('all',15,True),
+       collect_compiler_stats('all',15),
 
        # Use `+RTS -G1` for more stable residency measurements. Note [residency].
        extra_hc_opts('+RTS -G1 -RTS')
@@ -186,40 +186,40 @@ test('T9675',
 
 test('T9872a',
      [ only_ways(['normal']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
       ],
      compile_fail,
      [''])
 
 test('T9872b',
      [ only_ways(['normal']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
       ],
      compile_fail,
      [''])
 test('T9872c',
      [ only_ways(['normal']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
       ],
      compile_fail,
      [''])
 test('T9872d',
      [ only_ways(['normal']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
       ],
      compile,
      [''])
 
 test('T9961',
      [ only_ways(['normal']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
       ],
      compile,
      ['-O'])
 
 test('T9233',
     [ only_ways(['normal']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
       extra_clean(['T9233a.hi', 'T9233a.o'])
     ],
     multimod_compile,
@@ -227,7 +227,7 @@ test('T9233',
 
 test('T10370',
      [ only_ways(['optasm']),
-       collect_stats(['max_bytes_used','peak_megabytes_allocated'],15,True),
+       collect_compiler_stats(['max_bytes_used','peak_megabytes_allocated'],15),
        # Use `+RTS -G1` for more stable residency measurements. Note [residency].
        extra_hc_opts('+RTS -G1 -RTS')
      ],
@@ -235,14 +235,14 @@ test('T10370',
      [''])
 
 test('T10547',
-     [ collect_stats('bytes allocated',20,True),
+     [ collect_compiler_stats('bytes allocated',20),
      ],
      compile_fail,
      ['-fprint-expanded-synonyms'])
 
 test('T12227',
      [ only_ways(['normal']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
      ],
      compile,
      # Use `-M1G` to prevent memory thrashing with ghc-8.0.1.
@@ -250,21 +250,21 @@ test('T12227',
 
 test('T12425',
      [ only_ways(['optasm']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
      ],
      compile,
      [''])
 
 test('T12234',
      [ only_ways(['optasm']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
      ],
      compile,
      [''])
 
 test('T12545',
      [ only_ways(['normal']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
        extra_clean(['T12545a.hi', 'T12545a.o'])
      ],
      multimod_compile,
@@ -272,39 +272,39 @@ test('T12545',
 
 test('T13035',
      [ only_ways(['normal']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
      ],
      compile,
      [''] )
 
 test('T13056',
      [ only_ways(['optasm']),
-       collect_stats('bytes allocated',10,True),
+       collect_compiler_stats('bytes allocated',10),
      ],
      compile,
      ['-O1'])
 
 test('T12707',
-     [ collect_stats('bytes allocated',5,True),
+     [ collect_compiler_stats('bytes allocated',5),
      ],
      compile,
      [''])
 
 test('T12150',
      [ only_ways(['optasm']),
-       collect_stats('bytes allocated',5,True),
+       collect_compiler_stats('bytes allocated',5),
      ],
     compile,
      [''])
 
 test('T13379',
-     [ collect_stats('bytes allocated',10,True),
+     [ collect_compiler_stats('bytes allocated',10),
      ],
      compile,
      [''])
 
 test('MultiLayerModules',
-     [ collect_stats('bytes allocated',10,True),
+     [ collect_compiler_stats('bytes allocated',10),
        pre_cmd('./genMultiLayerModules'),
        extra_files(['genMultiLayerModules']),
      ],
@@ -312,7 +312,7 @@ test('MultiLayerModules',
      ['MultiLayerModules', '-v0'])
 
 test('T13701',
-     [ collect_stats('bytes allocated',10,True),
+     [ collect_compiler_stats('bytes allocated',10),
        pre_cmd('./genT13701'),
        extra_files(['genT13701']),
      ],
@@ -320,7 +320,7 @@ test('T13701',
      ['T13701', '-v0'])
 
 test('T13719',
-     [ collect_stats('bytes allocated',10,True),
+     [ collect_compiler_stats('bytes allocated',10),
        pre_cmd('./genT13719'),
        extra_files(['genT13719']),
      ],
@@ -330,13 +330,13 @@ test('T13719',
 test('Naperian',
      [ reqlib('vector'),
        only_ways(['optasm']),
-       collect_stats('bytes allocated',10,True),
+       collect_compiler_stats('bytes allocated',10),
      ],
      compile,
      [''])
 
 test ('T9630',
-      [ collect_stats('bytes allocated',15,True),
+      [ collect_compiler_stats('bytes allocated',15),
       extra_clean(['T9630a.hi', 'T9630a.o'])
       ],
       multimod_compile,
diff --git a/testsuite/tests/simplCore/should_compile/all.T b/testsuite/tests/simplCore/should_compile/all.T
index 82a9cc9a52..2b96378c70 100644
--- a/testsuite/tests/simplCore/should_compile/all.T
+++ b/testsuite/tests/simplCore/should_compile/all.T
@@ -152,7 +152,7 @@ test('T7702',
       # we say 18mb peak allocated +/- 70% because other compiler flags have
       # a large effect on allocation which is hard to separate from the
       # allocation done by the plugin... but a regression allocates > 90mb
-      collect_stats('peak_megabytes_allocated',70,True),
+      collect_compiler_stats('peak_megabytes_allocated',70),
      ],
      compile,
      ['-v0 -package-db T7702plugin/pkg.T7702/local.package.conf -fplugin T7702Plugin -package T7702plugin ' + config.plugin_way_flags])