testsuite: Make performance metric summary more readable

Along with some refactoring.

4 files changed, 53 insertions, 20 deletions

diff --git a/testsuite/driver/perf_notes.py b/testsuite/driver/perf_notes.py
index cebb8f9815..dcaa3d11ca 100644
--- a/testsuite/driver/perf_notes.py
+++ b/testsuite/driver/perf_notes.py
@@ -70,9 +70,16 @@ Baseline = NamedTuple('Baseline', [('perfStat', PerfStat),
                                    ('commitDepth', int)])
 
 class MetricChange(Enum):
+    # The metric appears to have no baseline and is presumably a new test.
     NewMetric = 'NewMetric'
+
+    # The metric has not changed.
     NoChange = 'NoChange'
+
+    # The metric increased.
     Increase = 'Increase'
+
+    # The metric decreased.
     Decrease = 'Decrease'
 
 AllowedPerfChange = NamedTuple('AllowedPerfChange',
@@ -188,7 +195,7 @@ def parse_allowed_perf_changes(commitMsg: str
 # Calculates a suggested string to append to the git commit in order to accept the
 # given changes.
 # changes: [(MetricChange, PerfStat)]
-def allow_changes_string(changes: List[Tuple[MetricChange, PerfStat, Any]]
+def allow_changes_string(changes: List[Tuple[MetricChange, PerfStat]]
                          ) -> str:
     Dec = MetricChange.Decrease
     Inc = MetricChange.Increase
@@ -198,7 +205,7 @@ def allow_changes_string(changes: List[Tuple[MetricChange, PerfStat, Any]]
 
     # Map tests to a map from change direction to metrics.
     test_to_dir_to_metrics = {} # type: Dict[TestName, Dict[MetricChange, List[MetricName]]]
-    for (change, perf_stat, _baseline) in changes:
+    for (change, perf_stat) in changes:
         change_dir_to_metrics = test_to_dir_to_metrics.setdefault(perf_stat.test, { Inc: [], Dec: [] })
         change_dir_to_metrics[change].append(perf_stat.metric)
 
diff --git a/testsuite/driver/runtests.py b/testsuite/driver/runtests.py
index 9b26e1bf75..0b1f8d69fe 100644
--- a/testsuite/driver/runtests.py
+++ b/testsuite/driver/runtests.py
@@ -24,7 +24,8 @@ import traceback
 import subprocess
 
 from testutil import getStdout, Watcher, str_warn, str_info
-from testglobals import getConfig, ghc_env, getTestRun, TestConfig, TestOptions, brokens
+from testglobals import getConfig, ghc_env, getTestRun, TestConfig, \
+                        TestOptions, brokens, PerfMetric
 from perf_notes import MetricChange, inside_git_repo, is_worktree_dirty, format_perf_stat
 from junit import junit
 import term_color
@@ -333,6 +334,33 @@ def cleanup_and_exit(exitcode):
         shutil.rmtree(tempdir, ignore_errors=True)
     exit(exitcode)
 
+def tabulate_metrics(metrics: List[PerfMetric]) -> None:
+    direction_strings = {
+        MetricChange.NewMetric: colored(Color.BLUE,  "new"),
+        MetricChange.NoChange:  colored(Color.WHITE, "unchanged"),
+        MetricChange.Increase:  colored(Color.RED,   "increased"),
+        MetricChange.Decrease:  colored(Color.GREEN, "decreased")
+    }
+
+    for metric in sorted(metrics, key=lambda m: (m.stat.test, m.stat.way)):
+        print("{test:24}  {metric:40}  {value:15.3f}".format(
+            test = "{}({})".format(metric.stat.test, metric.stat.way),
+            metric = metric.stat.metric,
+            value = metric.stat.value
+        ))
+        if metric.baseline is not None:
+            val0 = metric.baseline.perfStat.value
+            val1 = metric.stat.value
+            rel = 100 * (val1 - val0) / val0
+            print("{space:24}  {herald:40}  {value:15.3f}  [{direction} {rel:2.1f}%]".format(
+                space = "",
+                herald = "(baseline @ HEAD~{depth})".format(
+                    depth = metric.baseline.commitDepth),
+                value = val0,
+                direction = direction_strings[metric.change],
+                rel = abs(rel)
+            ))
+
 # First collect all the tests to be run
 t_files_ok = True
 for file in t_files:
@@ -396,20 +424,12 @@ else:
     sys.stdout.flush()
 
     # Dump metrics data.
+    print("\nPerformance Metrics (test environment: {}):\n".format(config.test_env))
     if any(t.metrics):
-        print("\nPerformance Metrics:\n")
-        for (change, stat, baseline) in t.metrics:
-            if baseline is None:
-                print("{stat} [{direction}]".format(
-                    stat = format_perf_stat(stat),
-                    direction = str(change)))
-            else:
-                print("{stat} [{direction} from ({baselineStat}) @ HEAD~{depth}]".format(
-                    stat = format_perf_stat(stat),
-                    baselineStat = format_perf_stat(baseline.perfStat, ", "),
-                    direction = str(change),
-                    depth = baseline.commitDepth))
-        print("")
+        tabulate_metrics(t.metrics)
+    else:
+        print("\nNone collected.")
+    print("")
 
     # Warn if had to force skip perf tests (see Note force skip perf tests).
     spacing = "       "
@@ -454,7 +474,7 @@ else:
         print(str_info("Some stats have changed") + " If this is expected, " + \
             "allow changes by appending the git commit message with this:")
         print('-' * 25)
-        print(Perf.allow_changes_string(t.metrics))
+        print(Perf.allow_changes_string([(m.change, m.stat) for m in t.metrics]))
         print('-' * 25)
 
     summary(t, sys.stdout, config.no_print_summary, config.supports_colors)
diff --git a/testsuite/driver/testglobals.py b/testsuite/driver/testglobals.py
index 2393247b22..adf4112ca6 100644
--- a/testsuite/driver/testglobals.py
+++ b/testsuite/driver/testglobals.py
@@ -216,6 +216,12 @@ class TestResult:
         self.stdout = stdout
         self.stderr = stderr
 
+# A performance metric measured in this test run.
+PerfMetric = NamedTuple('PerfMetric',
+                        [('change', MetricChange),
+                         ('stat', PerfStat),
+                         ('baseline', Optional[Baseline]) ])
+
 class TestRun:
    def __init__(self) -> None:
        self.start_time = None # type: Optional[datetime]
@@ -243,7 +249,7 @@ class TestRun:
        # [(change, PerfStat)] where change is one of the  MetricChange
        # constants: NewMetric, NoChange, Increase, Decrease.
        # NewMetric happens when the previous git commit has no metric recorded.
-       self.metrics = [] # type: List[Tuple[MetricChange, PerfStat, Optional[Baseline]]]
+       self.metrics = [] # type: List[PerfMetric]
 
 global t
 t = TestRun()
diff --git a/testsuite/driver/testlib.py b/testsuite/driver/testlib.py
index 9d24951361..b72fd4fac5 100644
--- a/testsuite/driver/testlib.py
+++ b/testsuite/driver/testlib.py
@@ -19,7 +19,7 @@ import collections
 import subprocess
 
 from testglobals import config, ghc_env, default_testopts, brokens, t, \
-                        TestRun, TestResult, TestOptions
+                        TestRun, TestResult, TestOptions, PerfMetric
 from testutil import strip_quotes, lndir, link_or_copy_file, passed, \
                      failBecause, testing_metrics, \
                      PassFail
@@ -1429,7 +1429,7 @@ def check_stats(name: TestName,
                         tolerance_dev,
                         config.allowed_perf_changes,
                         config.verbose >= 4)
-                t.metrics.append((change, perf_stat, baseline))
+                t.metrics.append(PerfMetric(change=change, stat=perf_stat, baseline=baseline))
 
             # If any metric fails then the test fails.
             # Note, the remaining metrics are still run so that