Add support for manually running the real-time benchmark on PRs #19265
Conversation
kocsismate
force-pushed
the
benchmark-manual-workflow
branch
from
3de527f to
a233037
Compare
kocsismate
changed the title
kocsismate
force-pushed
the
benchmark-manual-workflow
branch
from
a233037 to
64751de
Compare
| required: true | ||
| type: string | ||
| commit: | ||
| description: 'Commit SHA that is going to be benchmarked (e.g. "123456789a")' |
There was a problem hiding this comment.
Why not just use ref and baseline_ref? This can be either a branch, tag, commit, etc.
There was a problem hiding this comment.
Possibly, this could even request just the PR number instead of repository, branch, commit, baseline_commit; as we can determine all of these from the PR.
There was a problem hiding this comment.
The branch and the commit is needed for various reasons, mostly displaying them. However, I love the idea of gathering these info automatically from github CLI 😍 I'm about to finish the implementation.
One caveat is that originally I planned that the workflow could be run for branches, tags, or even for master. A required PR number input will prevent these use-cases, but I think the 99% of usecases are still covered, so I'm fine with the loss.
| fetch-depth: 100 | ||
| path: 'php-version-benchmarks/tmp/php_master' | ||
| path: 'php-version-benchmarks/tmp/php_${{ env.BRANCH }}' |
There was a problem hiding this comment.
Could probably drop the dynamic path suffix _${{ env.BRANCH }} here.
There was a problem hiding this comment.
I left the suffix there so that the directory name matches the PHP ID (master for scheduled, benchmarked for the manual flow). It's probably not strictly needed but this makes it easier to see what PHP version the directory relates to.
|
@iluuu1994 Thanks for looking into it! I have managed to start testing the manual workflow on my own repo. I implemented some fixes there, but unfortunately a separate issue (github rate limit) blocks it from finishing successfully, so I have to take care of this problem first. Afterwards, I'll update this PR :) |
|
Successful test workflow run: https://github.com/kocsismate/php-src/actions/runs/16633526468/job/47068814288 The PR is updated. (I fixed the bug with not benchmarking the baseline version without JIT after the comment was made) |
kocsismate
force-pushed
the
benchmark-manual-workflow
branch
from
6e2de73 to
a70d9b1
Compare
kocsismate
force-pushed
the
benchmark-manual-workflow
branch
from
a70d9b1 to
965368a
Compare
|
@iluuu1994 Can I ask you to trigger the workflow using this branch in order to test it on php-src as well? And I think you could supply this PR as input if you don't have any better PR that would benefit from some benchmark. |
|
@kocsismate I don't think I can. The workflow trigger button won't show up until the action is configured in the default branch. |
|
Ah yes, I forgot that the manual workflow setting needs to be merged first 🫤 |
|
Let me merge this, because it would (hopefully) fix the scheduled benchmark that will run in a few hours. It has been failing for quite a few days now. I happily accept after the fact reviews anytime! |
No description provided.