-wip/t-12758_circleci_push_perf_notes

7 files changed, 88 insertions, 57 deletions

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 7401ece8e2..3397cabb9e 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -65,44 +65,21 @@ aliases:
       command: |
         cabal update
         hadrian/build.sh -j`mk/detect-cpu-count.sh`
-
-
-  - &test_push
-    run:
-      name: Test Push
-      command: |
-        # Add git.haskell.org as a known host.
-        echo "|1|F3mPVCE55+KfApNIMYQ3Dv39sGE=|1bRkvJEJhAN2R0LE/lAjFCEJGl0= ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBBUZS9jGBkE5UzpSo6irnIgcQcfzvbuIOsFc8+N61FwtZncRntbaKPuUimOFPgeaUZLl6Iajz6IIs7aduU0/v+I=" >> ~/.ssh/known_hosts
-        echo "|1|2VUMjYSRVpT2qJPA0rA9ap9xILY=|5OThkI4ED9V0J+Es7D5FOD55Klk= ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC+3TLluLAO4lkW60W+N2DFkS+WoRFGqLwHzgd1ifxG9TIm31wChPY3E/hgMnJmgGqWCF4UDUemmyCycEaL7FtKfzjTAclg9EfpQnozyE3T5hIo2WL7SN5O8ttG/bYGuDnn14jLnWwJyN4oz/znWFiDG9e2Oc9YFNlQ+PK8ae5xR4gqBB7EOoj9J1EiPqG2OXRr5Mei3TLsRDU6fnz/e4oFJpKWWeN6M63oePv0qoaGjxcrATZUWsuWrxVMmYo9kP1xRuFJbAUw2m4uVP+793SW1zxySi1HBMtJG+gCDdZZSwYbkV1hassLWBHv1qPttncfX8Zek3Z3VolaTmfWJTo9" >> ~/.ssh/known_hosts
-
-        # Make fake note.
-        git notes --ref notes_test add -m "Test01" HEAD
-
-        # Push the git notes.
-        git push git@git.haskell.org:ghc refs/notes/notes_test
-
-
-
-
   - &test
     run:
       name: Test
       command: |
         mkdir -p test-results
-        make test                  TEST="T8766"                              THREADS=`mk/detect-cpu-count.sh` SKIP_PERF_TESTS=$SKIP_PERF_TESTS TEST_ENV=$TEST_ENV JUNIT_FILE=../../test-results/junit.xml
+        METRICS_FILE=$(mktemp)
+        echo "export METRICS_FILE=$METRICS_FILE" >> $BASH_ENV
+        make test THREADS=`mk/detect-cpu-count.sh` SKIP_PERF_TESTS=$SKIP_PERF_TESTS TEST_ENV=$TEST_ENV JUNIT_FILE=../../test-results/junit.xml METRICS_FILE=$METRICS_FILE
   - &store_test_results
     store_test_results:
       path: test-results
   - &push_perf_note
     run:
       name: Push Performance Git Notes
-      command: |
-        # Add git.haskell.org as a known host.
-        echo "|1|F3mPVCE55+KfApNIMYQ3Dv39sGE=|1bRkvJEJhAN2R0LE/lAjFCEJGl0= ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBBUZS9jGBkE5UzpSo6irnIgcQcfzvbuIOsFc8+N61FwtZncRntbaKPuUimOFPgeaUZLl6Iajz6IIs7aduU0/v+I=" >> ~/.ssh/known_hosts
-        echo "|1|2VUMjYSRVpT2qJPA0rA9ap9xILY=|5OThkI4ED9V0J+Es7D5FOD55Klk= ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC+3TLluLAO4lkW60W+N2DFkS+WoRFGqLwHzgd1ifxG9TIm31wChPY3E/hgMnJmgGqWCF4UDUemmyCycEaL7FtKfzjTAclg9EfpQnozyE3T5hIo2WL7SN5O8ttG/bYGuDnn14jLnWwJyN4oz/znWFiDG9e2Oc9YFNlQ+PK8ae5xR4gqBB7EOoj9J1EiPqG2OXRr5Mei3TLsRDU6fnz/e4oFJpKWWeN6M63oePv0qoaGjxcrATZUWsuWrxVMmYo9kP1xRuFJbAUw2m4uVP+793SW1zxySi1HBMtJG+gCDdZZSwYbkV1hassLWBHv1qPttncfX8Zek3Z3VolaTmfWJTo9" >> ~/.ssh/known_hosts
-
-        # Push the git notes.
-        git push git@git.haskell.org:ghc refs/notes/perf
+      command: .circleci/push-test-metrics.sh
   - &slowtest
     run:
       name: Full Test
@@ -145,20 +122,12 @@ jobs:
       - *submodules
       - *boot
       - *configure_unix
-      #
-      #
-      #
-      #
-      - *test_push
-      #
-      #
-      #
-      #- *make
-      #- *bindist
-      #- *storeartifacts
-      #- *test
-      #- *store_test_results
-      #- *push_perf_note
+      - *make
+      - *bindist
+      - *storeartifacts
+      - *test
+      - *store_test_results
+      - *push_perf_note
 
   "validate-x86_64-freebsd":
     resource_class: xlarge
@@ -363,11 +332,11 @@ workflows:
         *trigger_on_tags
     # FreeBSD disabled: https://github.com/haskell/unix/issues/102
     # - validate-x86_64-freebsd
-#    - validate-x86_64-darwin:
-#        *trigger_on_tags
-#    - validate-x86_64-linux-llvm
-#    - validate-i386-linux:
-#        *trigger_on_tags
+    - validate-x86_64-darwin:
+        *trigger_on_tags
+    - validate-x86_64-linux-llvm
+    - validate-i386-linux:
+        *trigger_on_tags
 #    - validate-hadrian-x86_64-linux
 #    - validate-x86_64-fedora:
 #        *trigger_on_tags
diff --git a/.circleci/push-test-metrics.sh b/.circleci/push-test-metrics.sh
new file mode 100755
index 0000000000..302e4bf39e
--- /dev/null
+++ b/.circleci/push-test-metrics.sh
@@ -0,0 +1,46 @@
+#!/usr/bin/env bash
+# vim: sw=2 et
+set -euo pipefail
+
+fail() {
+  echo "ERROR: $*" >&2
+  exit 1
+}
+
+GHC_ORIGIN=git@git.haskell.org:ghc
+
+# Add git.haskell.org as a known host.
+echo "|1|F3mPVCE55+KfApNIMYQ3Dv39sGE=|1bRkvJEJhAN2R0LE/lAjFCEJGl0= ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBBUZS9jGBkE5UzpSo6irnIgcQcfzvbuIOsFc8+N61FwtZncRntbaKPuUimOFPgeaUZLl6Iajz6IIs7aduU0/v+I=" >> ~/.ssh/known_hosts
+echo "|1|2VUMjYSRVpT2qJPA0rA9ap9xILY=|5OThkI4ED9V0J+Es7D5FOD55Klk= ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC+3TLluLAO4lkW60W+N2DFkS+WoRFGqLwHzgd1ifxG9TIm31wChPY3E/hgMnJmgGqWCF4UDUemmyCycEaL7FtKfzjTAclg9EfpQnozyE3T5hIo2WL7SN5O8ttG/bYGuDnn14jLnWwJyN4oz/znWFiDG9e2Oc9YFNlQ+PK8ae5xR4gqBB7EOoj9J1EiPqG2OXRr5Mei3TLsRDU6fnz/e4oFJpKWWeN6M63oePv0qoaGjxcrATZUWsuWrxVMmYo9kP1xRuFJbAUw2m4uVP+793SW1zxySi1HBMtJG+gCDdZZSwYbkV1hassLWBHv1qPttncfX8Zek3Z3VolaTmfWJTo9" >> ~/.ssh/known_hosts
+
+# Check that a git notes dont already exist.
+# This is a percausion as we reset refs/notes/perf and we want to avoid data loss.
+if [ $(git notes --ref=perf list | wc -l) -ne 0 ]
+then
+  fail "Found an existing git note on HEAD. Expected no git note."
+fi
+
+# Assert that the METRICS_FILE exists and can be read.
+if [ "$METRICS_FILE" = "" ] || ! [ -r $METRICS_FILE ]
+then
+  fail "Metrics file not found: $METRICS_FILE"
+fi
+
+# Reset the git notes and append the metrics file to the notes, then push and return the result.
+# This is favoured over a git notes merge as it avoids potential data loss/duplication from the merge strategy.
+function reset_append_note_push {
+  git fetch -f $GHC_ORIGIN refs/notes/perf:refs/notes/perf || true
+  echo "git notes --ref=perf append -F $METRICS_FILE HEAD"
+  git notes --ref=perf append -F $METRICS_FILE HEAD
+  git push $GHC_ORIGIN refs/notes/perf
+}
+
+# Push the metrics file as a git note. This may fail if another task pushes a note first. In that case
+# the latest note is fetched and appended.
+MAX_RETRY=20
+until reset_append_note_push || [ MAX_RETRY = 0 ]
+do
+  ((MAX_RETRY--))
+  echo ""
+  echo "Failed to push git notes. Fetching, appending, and retrying..."
+done
\ No newline at end of file
diff --git a/testsuite/driver/perf_notes.py b/testsuite/driver/perf_notes.py
index fd5ee3d638..f162164e3e 100644
--- a/testsuite/driver/perf_notes.py
+++ b/testsuite/driver/perf_notes.py
@@ -170,21 +170,25 @@ def allow_changes_string(changes):
 
     return '\n\n'.join(msgs)
 
+# Formats a list of metrics into a string. Used e.g. to save metrics to a file or git note.
+def format_perf_stat(stats):
+    # If a single stat, convert to a singleton list.
+    if not isinstance(stats, list):
+        stats = [stats]
+
+    return "\n".join(["\t".join([str(stat_val) for stat_val in stat]) for stat in stats])
+
 # Appends a list of metrics to the git note of the given commit.
 # Tries up to max_tries times to write to git notes should it fail for some reason.
 # Each retry will wait 1 second.
 # Returns True if the note was successfully appended.
 def append_perf_stat(stats, commit='HEAD', namespace='perf', max_tries=5):
-    # If a single stat, convert to a singleton list.
-    if not isinstance(stats, list):
-        stats = [stats]
-
     # Append to git note
     print('Appending ' + str(len(stats)) + ' stats to git notes.')
-    stats_str = "\n".join(["\t".join([str(stat_val) for stat_val in stat]) for stat in stats])
+    stats_str = format_perf_stat(stats)
     def try_append():
             try:
-                return subprocess.check_output(['git', 'notes', '--ref=perf', 'append', commit, '-m', stats_str])
+                return subprocess.check_output(['git', 'notes', '--ref=' + namespace, 'append', commit, '-m', stats_str])
             except subprocess.CalledProcessError:
                 return b'Git - fatal'
 
diff --git a/testsuite/driver/runtests.py b/testsuite/driver/runtests.py
index 6f59ff7f67..fb3fe6ad54 100644
--- a/testsuite/driver/runtests.py
+++ b/testsuite/driver/runtests.py
@@ -50,6 +50,7 @@ parser.add_argument("-e", action='append', help="A string to execute from the co
 parser.add_argument("--config-file", action="append", help="config file")
 parser.add_argument("--config", action='append', help="config field")
 parser.add_argument("--rootdir", action='append', help="root of tree containing tests (default: .)")
+parser.add_argument("--metrics-file", help="file in which to save (append) the performance test metrics. If omitted, git notes will be used.")
 parser.add_argument("--summary-file", help="file in which to save the (human-readable) summary")
 parser.add_argument("--no-print-summary", action="store_true", help="should we print the summary?")
 parser.add_argument("--only", action="append", help="just this test (can be give multiple --only= flags)")
@@ -82,6 +83,7 @@ all_ways = config.run_ways+config.compile_ways+config.other_ways
 if args.rootdir:
     config.rootdirs = args.rootdir
 
+config.metrics_file = args.metrics_file
 config.summary_file = args.summary_file
 config.no_print_summary = args.no_print_summary
 
@@ -367,7 +369,13 @@ else:
 
     summary(t, sys.stdout, config.no_print_summary, True)
 
-    Perf.append_perf_stat([stat for (_, stat) in t.metrics])
+    stats = [stat for (_, stat) in t.metrics]
+    if config.metrics_file:
+        print('Appending ' + str(len(stats)) + ' stats to file: ' + config.metrics_file)
+        with open(config.metrics_file, 'a') as file:
+            file.write("\n" + Perf.format_perf_stat(stats))
+    else:
+        Perf.append_perf_stat(stats)
 
     if config.summary_file:
         with open(config.summary_file, 'w') as file:
diff --git a/testsuite/driver/testglobals.py b/testsuite/driver/testglobals.py
index 6434d7a443..03a62503b4 100644
--- a/testsuite/driver/testglobals.py
+++ b/testsuite/driver/testglobals.py
@@ -31,6 +31,9 @@ class TestConfig:
         self.accept_platform = False
         self.accept_os = False
 
+        # File in which to save the performance metrics.
+        self.metrics_file = ''
+
         # File in which to save the summary
         self.summary_file = ''
 
diff --git a/testsuite/mk/test.mk b/testsuite/mk/test.mk
index 97c8c433ce..65e897d849 100644
--- a/testsuite/mk/test.mk
+++ b/testsuite/mk/test.mk
@@ -274,6 +274,10 @@ RUNTEST_OPTS +=  \
 
 RUNTEST_OPTS += -e "config.stage=$(GhcStage)"
 
+ifneq "$(METRICS_FILE)" ""
+RUNTEST_OPTS +=  \
+	--metrics-file "$(METRICS_FILE)"
+endif
 ifneq "$(JUNIT_FILE)" ""
 RUNTEST_OPTS +=  \
   --junit "$(JUNIT_FILE)"
diff --git a/testsuite/tests/perf/should_run/all.T b/testsuite/tests/perf/should_run/all.T
index cd586c0a37..0b70398e46 100644
--- a/testsuite/tests/perf/should_run/all.T
+++ b/testsuite/tests/perf/should_run/all.T
@@ -358,10 +358,7 @@ test('T15426',
     ['-O2'])
 
 test('T15578',
-    [stats_num_field('bytes allocated',
-                    [ (wordsize(64), 800041456, 5) ]),
-                    # 2018-09-07     800041456   Improvements from #15578
-                    # initial      42400041456
+    [collect_stats('bytes allocated', 5),
      only_ways(['normal'])],
     compile_and_run,
     ['-O2'])