Performance tests: Make them faster #30021
Comments
I guess this whole proposal is based on this premise but unfortunately I don't really think it's a good idea. Two jobs run on CI doesn't guarantee the same environment (same CPU power, same memory..) which means the results are not correct across jobs, this is why we need to run both branches on the same CI job. Actually, even the way we iterate 3 times is not optimal. Instead of doing:
We should be doing
This would ensure the numbers are stable enough and the environments match as closely as possible. |
another problem I can think of with the proposed approach is that a PR changing the "performance e2e test file" itself (so changing the rules) would result in completely inconsistent results across branches. |
Those are some good points. While reading this, I wondered: do we really need to run the performance tests for every single PR? |
ockham commentedMar 19, 2021
aka Increase performance tests performance😄
The perf tests are currently taking around 40 minutes to run on a given PR. That time is spent on cloning the git repo, checking out the
trunk
branch,npm install && npm run build:packages
, running the tests, and then switching to the current branch and repeating the install/build/tests steps (seeperformance.js
).I think we can cut this down significantly by being a bit smarter:
push
totrunk
.trunk
, we use GH's API to find the latest run of the performance workflow fortrunk
that completed successfully, and download the test results artifact. We then use that, and the artifact from the current branch, to create the table of test results.Since we'd only run the perf tests for the current branch on each PR (while fetching the results for
trunk
from GH), I think this should cut down the perf workflow's runtime by 50% -- currently around 20 minutes. Probably even a bit less than that, since we'd skip a bit of overhead (repo cloning).In a second step, this can probably extended for the release case (comparing the current and previous GB release branches, and the current WordPress
wp/
release branch). The major hurdle there might be the lifespan of build artifacts: It's possible that thewp/
release branch artifact is quite old, and has potentially been removed by GH at that time. We might need a fallback to recreate it on the fly.This is notably a CI-first approach; I'm not quite sure how to best backport it to the locally run
bin/plugin/cli.js perf
(performance.js
) script. Arguably, it doesn't make sense to download test results created on a CI machine to compare them to results created locally on a developer's system, since those machines' specs might totally skew the result. Right now, I'm leaning towards just changing the CI workflow (performance.yml
), and leaving the local script as-is -- potentially changing it later. One option would be to dumb down the local script to only compare test results fetched from CI (rather than running the actual perf tests itself).This could be a fun little project to learn a bit more about GitHub Actions. I wouldn't mind mentoring someone doing this, rather than implementing it myself. Any takers?😬 cc/ @david-szabo97 @rafaelgalani
Curious to hear thoughts from @WordPress/gutenberg-core.
The text was updated successfully, but these errors were encountered: