From 17b96b1c1b95625e799c5b550cd4e34e33615dc5 Mon Sep 17 00:00:00 2001
From: Fabrice Normandin <fabrice.normandin@gmail.com>
Date: Thu, 3 Aug 2023 16:56:34 -0400
Subject: [PATCH] Add Performance Regression tests to CI (#282)

* Add a benchmark_comment workflow on pull requests

Signed-off-by: Fabrice Normandin <normandf@mila.quebec>

* Tweak the test parameterization

Signed-off-by: Fabrice Normandin <normandf@mila.quebec>

* Fix tiny bug in perf test for Py3.7

Signed-off-by: Fabrice Normandin <normandf@mila.quebec>

---------

Signed-off-by: Fabrice Normandin <normandf@mila.quebec>
---
 .github/workflows/benchmark.yml         | 17 +++++++---
 .github/workflows/benchmark_comment.yml | 41 +++++++++++++++++++++++++
 test/test_performance.py                | 26 ++++++++++++++++
 3 files changed, 80 insertions(+), 4 deletions(-)
 create mode 100644 .github/workflows/benchmark_comment.yml

diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml
index a4e30f9e..4fa902f1 100644
--- a/.github/workflows/benchmark.yml
+++ b/.github/workflows/benchmark.yml
@@ -15,7 +15,7 @@ permissions:
 
 jobs:
   benchmark:
-    name: Run pytest-benchmark benchmark example
+    name: Run benchmark-action
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
@@ -24,11 +24,18 @@ jobs:
         with:
           python-version: 3.11
           cache: "pip"
+
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
           pip install -e .[all]
 
+      - name: Download previous benchmark data
+        uses: actions/cache@v3
+        with:
+          path: ./cache
+          key: ${{ runner.os }}-benchmark
+
       - name: Run benchmark
         run: |
           pytest --benchmark-only --benchmark-json=.benchmark_output.json
@@ -41,15 +48,17 @@ jobs:
           # Where the output from the benchmark tool is stored
           output-file-path: .benchmark_output.json
           # # Where the previous data file is stored
-          # external-data-json-path: ./cache/benchmark-data.json
+          external-data-json-path: ./cache/benchmark-master.json
           # Use personal access token instead of GITHUB_TOKEN due to https://github.community/t/github-action-not-triggering-gh-pages-upon-push/16096
           github-token: ${{ secrets.GITHUB_TOKEN }}
           # NOTE: auto-push must be false when external-data-json-path is set since this action
           # reads/writes the given JSON file and never pushes to remote
-          auto-push: true
+          auto-push: false
           # Show alert with commit comment on detecting possible performance regression
-          alert-threshold: '200%'
+          alert-threshold: '150%'
           comment-on-alert: true
+          # Enable Job Summary for PRs
+          summary-always: true
           # Workflow will fail when an alert happens
           fail-on-alert: true
           alert-comment-cc-users: '@lebrice'
diff --git a/.github/workflows/benchmark_comment.yml b/.github/workflows/benchmark_comment.yml
new file mode 100644
index 00000000..02e4471a
--- /dev/null
+++ b/.github/workflows/benchmark_comment.yml
@@ -0,0 +1,41 @@
+name: Compare Performance with Master
+on:
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  benchmark:
+    name: Run pytest-benchmark
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up Python 3.11
+        uses: actions/setup-python@v3
+        with:
+          python-version: 3.11
+          cache: "pip"
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -e .[all]
+
+      - name: Unit tests with Pytest
+        run: |
+          pytest --benchmark-only --benchmark-json=benchmark_results.json
+
+      - name: Download artifact
+        uses: dawidd6/action-download-artifact@v2
+        with:
+          github_token: ${{ github.token }}
+          workflow: upload.yml
+          name: benchmark_results
+          path: old_benchmark
+          commit: ${{github.event.pull_request.base.sha}}
+        continue-on-error: true
+
+      - name: Run the action
+        uses: nils-braun/pytest-benchmark-commenter@v2
+        with:
+          benchmark-file: benchmark_results.json
+          comparison-benchmark-file: "old_benchmark/benchmark_results.json"
\ No newline at end of file
diff --git a/test/test_performance.py b/test/test_performance.py
index 566a8a5d..273e45bd 100644
--- a/test/test_performance.py
+++ b/test/test_performance.py
@@ -1,5 +1,6 @@
 import functools
 import importlib
+from pathlib import Path
 import sys
 from typing import Callable, TypeVar
 import pytest
@@ -48,6 +49,9 @@ def test_import_performance(benchmark: BenchmarkFixture):
     benchmark(call_before(unimport_sp, import_sp))
 
 
+@pytest.mark.benchmark(
+    group="parse",
+)
 def test_parse_performance(benchmark: BenchmarkFixture):
     import simple_parsing as sp
     from test.nesting.example_use_cases import HyperParameters
@@ -57,3 +61,25 @@ def test_parse_performance(benchmark: BenchmarkFixture):
         HyperParameters,
         args="--age_group.num_layers 5 --age_group.num_units 65 ",
     )
+
+
+@pytest.mark.benchmark(
+    group="serialization",
+)
+@pytest.mark.parametrize("filetype", [".yaml", ".json", ".pkl"])
+def test_serialization_performance(benchmark: BenchmarkFixture, tmp_path: Path, filetype: str):
+    from simple_parsing.helpers.serialization import save, load
+    from test.test_huggingface_compat import TrainingArguments
+
+    args = TrainingArguments()
+    path = (tmp_path / "bob").with_suffix(filetype)
+
+    def save_and_load():
+        clear_lru_caches()
+        # NOTE: can't just use unlink(missing_ok=True) since python3.7 doesn't have it.
+        if path.exists():
+            path.unlink()
+        save(args, path)
+        assert load(TrainingArguments, path) == args
+
+    benchmark(save_and_load)