From d7484f6643198e18a3f4f7fc654ffbc8be613ac4 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 3 Oct 2024 20:43:45 -0700 Subject: [PATCH 1/3] add CI to detect performance regressions Compares two release builds of cpp-linter binary: 1. the previous commit (for push events) or the base branch of a PR 2. the newest commit on the branch 3. the latest v1.x release of the pure-python cpp-linter package Caching is enabled to reduce CI runtime. Results are output to the CI workflow's job summary. This CI does not (currently) fail when a regression is detected. --- .github/workflows/perf-test.yml | 135 +++++++++++++++++++++++++++++ .github/workflows/perf_annotate.py | 64 ++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 .github/workflows/perf-test.yml create mode 100644 .github/workflows/perf_annotate.py diff --git a/.github/workflows/perf-test.yml b/.github/workflows/perf-test.yml new file mode 100644 index 00000000..8bb0093e --- /dev/null +++ b/.github/workflows/perf-test.yml @@ -0,0 +1,135 @@ +name: Performance Regression + +on: + push: + branches: [main] + paths: + - cpp-linter/src/** + - cpp-linter/Cargo.toml + - Cargo.toml + - Cargo.lock + - .github/workflows/perf-test.yml + - .github/workflows/bench.py + tags-ignore: ['*'] + pull_request: + branches: [main] + paths: + - cpp-linter/src/** + - cpp-linter/Cargo.toml + - Cargo.toml + - Cargo.lock + - .github/workflows/perf* +jobs: + build: + name: Build ${{ matrix.name }} + runs-on: ubuntu-latest + strategy: + matrix: + include: + - commit: ${{ github.sha }} + name: current + - commit: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} + name: previous + outputs: + cached-previous: ${{ steps.is-cached-previous.outputs.is-cached }} + cached-current: ${{ steps.is-cached-current.outputs.is-cached }} + steps: + - name: Checkout ${{ matrix.name }} + uses: actions/checkout@v4 + with: + ref: ${{ matrix.commit }} + - name: Cache base ref build + uses: actions/cache@v4 + id: cache + with: + key: bin-cache-${{ hashFiles('cpp-linter/src/**', 'Cargo.toml', 'Cargo.lock', 'cpp-linter/Cargo.toml') }} + path: target/release/cpp-linter + - name: Is previous cached? + if: matrix.name == 'previous' + id: is-cached-previous + run: echo "is-cached=${{ steps.cache.outputs.cache-hit }}" >> $GITHUB_OUTPUT + - name: Is current cached? + if: matrix.name == 'current' + id: is-cached-current + run: echo "is-cached=${{ steps.cache.outputs.cache-hit }}" >> $GITHUB_OUTPUT + - run: rustup update --no-self-update + if: steps.cache.outputs.cache-hit != 'true' + - run: cargo build --bin cpp-linter --release + if: steps.cache.outputs.cache-hit != 'true' + - name: Upload build artifact + uses: actions/upload-artifact@v4 + with: + name: ${{ matrix.name }} + path: target/release/cpp-linter + + benchmark: + name: Measure Performance Difference + needs: [build] + if: ${{ needs.build.outputs.cached-current != 'true' || needs.build.outputs.cached-previous != 'true' }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Checkout libgit2 + uses: actions/checkout@v4 + with: + repository: libgit2/libgit2 + ref: v1.8.1 + path: libgit2 + - name: Download built binaries + uses: actions/download-artifact@v4 + - name: Make binaries executable + run: chmod +x ./*/cpp-linter + - name: Generate compilation database + working-directory: libgit2 + run: | + mkdir build && cd build + cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON .. + - name: Install cargo-binstall + uses: cargo-bins/cargo-binstall@main + - name: Install hyperfine + run: cargo binstall -y hyperfine + - uses: actions/setup-python@v5 + with: + python-version: 3.x + - run: pip install 'cpp-linter < 2.0' + - name: Warmup and list files + env: + CPP_LINTER_COLOR: true + working-directory: libgit2 + # Use previous build for stability. This will + # - create the .cpp-linter_cache folder + # - list the files concerning the benchmark test + # NOTE: This does not actually invoke clang tools. + run: ../previous/cpp-linter -l 0 -p build -i='|!src/libgit2' -s="" -c="-*" -e c + - name: Run hyperfine tool + # using the generated compilation database, + # we will use cpp-linter (both builds) to scan libgit2 src/libgit2/**.c files. + working-directory: libgit2 + run: >- + hyperfine + --runs 2 + --style color + --export-markdown '${{ runner.temp }}/benchmark.md' + --export-json '${{ runner.temp }}/benchmark.json' + --command-name=previous-build + "../previous/cpp-linter -l 0 -p build -i='|!src/libgit2' -e c" + --command-name=current-build + "../current/cpp-linter -l 0 -p build -i='|!src/libgit2' -e c" + --command-name=pure-python + "cpp-linter -l false -j 0 -p build -i='|!src/libgit2' -e c" + - name: Append report to job summary + run: cat ${{ runner.temp }}/benchmark.md >> $GITHUB_STEP_SUMMARY + - name: Upload JSON results + uses: actions/upload-artifact@v4 + with: + name: benchmark-json + path: ${{ runner.temp }}/benchmark.json + - name: Annotate summary + run: python .github/workflows/perf_annotate.py "${{ runner.temp }}/benchmark.json" + + report-no-src-changes: + runs-on: ubuntu-latest + needs: [build] + if: needs.build.outputs.cached-current == 'true' && needs.build.outputs.cached-previous == 'true' + steps: + - run: echo "::notice title=No benchmark performed::No changes to cpp-linter source code detected." diff --git a/.github/workflows/perf_annotate.py b/.github/workflows/perf_annotate.py new file mode 100644 index 00000000..0384922d --- /dev/null +++ b/.github/workflows/perf_annotate.py @@ -0,0 +1,64 @@ +import argparse +import json +from os import environ +from pathlib import Path +from typing import List, Any, Dict + + +class Args(argparse.Namespace): + json_file: Path + + +def main(): + arg_parser = argparse.ArgumentParser() + arg_parser.add_argument("json_file", type=Path) + arg_parser.parse_args(namespace=Args) + + bench_json = Args.json_file.read_text(encoding="utf-8") + bench: List[Dict[str, Any]] = json.loads(bench_json)["results"] + + assert len(bench) == 3 + assert bench[0]["command"] == "previous-build" + assert bench[1]["command"] == "current-build" + assert bench[2]["command"] == "pure-python" + + old_mean: float = bench[0]["mean"] + new_mean: float = bench[1]["mean"] + + diff = round(new_mean - old_mean, 2) + scalar = round(new_mean / old_mean, 2) * 100 + + output = [] + if diff > 2: + output.extend( + [ + "> [!CAUTION]", + "> Detected a performance regression in new changes:", + ] + ) + elif diff < -2: + output.extend( + [ + "> [!TIP]", + "> Detected a performance improvement in new changes:", + ] + ) + else: + output.extend( + [ + "> [!NOTE]", + "> Determined a negligible difference in performance with new changes:", + ] + ) + output[-1] += f" {diff}s ({scalar} %)" + annotation = "\n".join(output) + + if "GITHUB_STEP_SUMMARY" in environ: + with open(environ["GITHUB_STEP_SUMMARY"], "a") as summary: + summary.write(f"\n{annotation}\n") + else: + print(annotation) + + +if __name__ == "__main__": + main() From 14a3bc15d59621e6b962038e5200ba37a1d34a38 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 3 Oct 2024 21:12:33 -0700 Subject: [PATCH 2/3] review changes --- .github/workflows/perf_annotate.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/.github/workflows/perf_annotate.py b/.github/workflows/perf_annotate.py index 0384922d..072c9b0c 100644 --- a/.github/workflows/perf_annotate.py +++ b/.github/workflows/perf_annotate.py @@ -2,7 +2,7 @@ import json from os import environ from pathlib import Path -from typing import List, Any, Dict +from typing import List, Any, Dict, cast class Args(argparse.Namespace): @@ -18,15 +18,19 @@ def main(): bench: List[Dict[str, Any]] = json.loads(bench_json)["results"] assert len(bench) == 3 - assert bench[0]["command"] == "previous-build" - assert bench[1]["command"] == "current-build" - assert bench[2]["command"] == "pure-python" + old_mean, new_mean = (None, None) + for result in bench: + mean = cast(float, result["mean"]) + if result["command"] == "previous-build": + old_mean = mean + elif result["command"] == "current-build": + new_mean = mean - old_mean: float = bench[0]["mean"] - new_mean: float = bench[1]["mean"] + assert old_mean is not None, "benchmark report has no result for previous-build" + assert new_mean is not None, "benchmark report has no result for current-build" diff = round(new_mean - old_mean, 2) - scalar = round(new_mean / old_mean, 2) * 100 + scalar = int((new_mean - old_mean) / old_mean * 100) output = [] if diff > 2: From 409a238111f7f3e1510333c6a7264e1a24b91aeb Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 3 Oct 2024 21:36:25 -0700 Subject: [PATCH 3/3] validate cache --- .github/workflows/perf-test.yml | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/.github/workflows/perf-test.yml b/.github/workflows/perf-test.yml index 8bb0093e..f6ac7adb 100644 --- a/.github/workflows/perf-test.yml +++ b/.github/workflows/perf-test.yml @@ -31,8 +31,10 @@ jobs: - commit: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} name: previous outputs: - cached-previous: ${{ steps.is-cached-previous.outputs.is-cached }} - cached-current: ${{ steps.is-cached-current.outputs.is-cached }} + cached-previous: ${{ steps.is-cached-previous.outputs.is-cached == 'true' && steps.validate.outputs.cache-valid != 'false' }} + cached-current: ${{ steps.is-cached-current.outputs.is-cached == 'true' && steps.validate.outputs.cache-valid != 'false' }} + env: + BIN: target/release/cpp-linter steps: - name: Checkout ${{ matrix.name }} uses: actions/checkout@v4 @@ -43,29 +45,38 @@ jobs: id: cache with: key: bin-cache-${{ hashFiles('cpp-linter/src/**', 'Cargo.toml', 'Cargo.lock', 'cpp-linter/Cargo.toml') }} - path: target/release/cpp-linter + path: ${{ env.BIN }} - name: Is previous cached? if: matrix.name == 'previous' id: is-cached-previous - run: echo "is-cached=${{ steps.cache.outputs.cache-hit }}" >> $GITHUB_OUTPUT + run: echo "is-cached=${{ steps.cache.outputs.cache-hit }}" >> "$GITHUB_OUTPUT" - name: Is current cached? if: matrix.name == 'current' id: is-cached-current - run: echo "is-cached=${{ steps.cache.outputs.cache-hit }}" >> $GITHUB_OUTPUT + run: echo "is-cached=${{ steps.cache.outputs.cache-hit }}" >> "$GITHUB_OUTPUT" + - name: Validate cached binary + if: steps.cache.outputs.cache-hit == 'true' + id: validate + run: | + chmod +x ${{ env.BIN }} + if ! ${{ env.BIN }} version; then + echo "Cached binary is invalid, rebuilding..." + echo "cache-valid=false" >> "$GITHUB_OUTPUT" + fi - run: rustup update --no-self-update - if: steps.cache.outputs.cache-hit != 'true' + if: steps.cache.outputs.cache-hit != 'true' || steps.validate.outputs.cache-valid == 'false' - run: cargo build --bin cpp-linter --release - if: steps.cache.outputs.cache-hit != 'true' + if: steps.cache.outputs.cache-hit != 'true' || steps.validate.outputs.cache-valid == 'false' - name: Upload build artifact uses: actions/upload-artifact@v4 with: name: ${{ matrix.name }} - path: target/release/cpp-linter + path: ${{ env.BIN }} benchmark: name: Measure Performance Difference needs: [build] - if: ${{ needs.build.outputs.cached-current != 'true' || needs.build.outputs.cached-previous != 'true' }} + if: ${{ !needs.build.outputs.cached-current || !needs.build.outputs.cached-previous }} runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -118,7 +129,7 @@ jobs: --command-name=pure-python "cpp-linter -l false -j 0 -p build -i='|!src/libgit2' -e c" - name: Append report to job summary - run: cat ${{ runner.temp }}/benchmark.md >> $GITHUB_STEP_SUMMARY + run: cat ${{ runner.temp }}/benchmark.md >> "$GITHUB_STEP_SUMMARY" - name: Upload JSON results uses: actions/upload-artifact@v4 with: @@ -130,6 +141,6 @@ jobs: report-no-src-changes: runs-on: ubuntu-latest needs: [build] - if: needs.build.outputs.cached-current == 'true' && needs.build.outputs.cached-previous == 'true' + if: needs.build.outputs.cached-current && needs.build.outputs.cached-previous steps: - run: echo "::notice title=No benchmark performed::No changes to cpp-linter source code detected."