search

opensim-org / opensim-core

SimTK OpenSim C++ libraries and command-line applications, and Java/Python wrapping.
https://opensim.stanford.edu
Apache License 2.0
783 stars 316 forks source link

Adding automated performance testing to CI workflow #3686

Closed nickbianco closed 7 months ago

nickbianco commented 8 months ago

Summary of changes

This change adds automated performance testing for pull requests to our GitHub Actions CI workflow. The performance testing compares the target branch (usually main) to the PR source branch and posts a markdown table containing a summary of the performance testing results to the PR description body (see below for example). The performance testing can be triggered by including the string '[perf]' anywhere in the PR title or description body (not the commit message, the testing will be triggered for all commits pushed to the PR branch).

The automated testing relies on the new private repository with in opensim-org/opensim-perf, which is essentially a fork of the excellent adamkewley/osimperf. This new private repo allows to use self-hosted runners securely, since forks of public repos can modify workflow scripts to do very malicious things on our local machines (e.g., cryptomine Dogecoin).

Workflows in this repo will trigger a workflow called performance_analysis.yml in opensim-perf via the GitHub action Codex-/return-dispatch@v1.12.0. The sister action Codex-/await-remote-run@v1.11.0 lets us wait for the workflow on opensim-perf to finish before continuing. The third new action dawidd6/action-download-artifact@v3.0.0 allows us to download artifacts across repositories; these "handoffs" require the new personal access token PERF_DISPATCH_PAT which I've created and added as a secret variable to opensim-org.

I've also upgraded actions/upload-artifact to version 4 (i.e., actions/upload-artifact@v4), which now will upload artifacts for individual jobs immediately when they are available. We no longer have to wait for the full CI workflow to finish before the uploads occurs, which means that artifacts for jobs that passed will now be available even if other jobs fail.

One minor downside of this change is for PRs where performance testing is skipped, the new job Mac [target] will still show up as "skipped". This is a limitation of the GitHub Actions UI for now.

The end-to-end workflow looks something like this:

  1. A pull request (with [perf] somewhere in the title or body) triggers continuous_integration.yml.
  2. The CI builds both Mac [target] and Mac [source] and uploads the artifacts.
  3. The new job performance_analysis, which depends on the two Mac builds, triggers the workflow performance_analysis.yml on opensim-perf. It passes the commit hash and artifact names so the opensim-perf workflow knows where and what to download.
  4. The job performance_analysis now waits for opensim-perf to finish its workflow.
  5. opensim-perf downloads the two artifacts, unzips them, runs the performance testing suite, and uploads an artifact containing the results in the markdown table performance_results.md.
  6. The opensim-core workflow resumes, downloads the table, and posts it to the PR description body (or overwrites the current table if it exists).

Testing I've completed

All of it.

Looking for feedback on...

  1. Any security vulnerabilities I might have missed?
  2. Stability concerns about the custom actions from the GitHub Actions marketplace? They seem pretty stable, well-used, and well-documented, but the workflow wholly depends on them currently.

CHANGELOG.md (choose one)

Performance analysis

Test Name lhs [secs] σ [secs] rhs [secs] σ [secs] Speedup
Arm26 0.33 0.00 0.34 0.00 0.97
passive_dynamic_noanalysis 3.42 0.10 3.31 0.15 1.03
ToyDropLanding 0.13 0.00 0.13 0.00 0.99
Gait2354 0.34 0.00 0.34 0.00 1.00
passive_dynamic 4.13 0.00 4.14 0.01 1.00
ToyDropLanding_nomuscles 0.12 0.00 0.12 0.00 1.00
RajagopalModel 6.26 0.00 6.34 0.01 0.99

This change is Reviewable

nickbianco commented 7 months ago

Thanks @aymanhab!