-
Notifications
You must be signed in to change notification settings - Fork 801
[CI][Bench] Refactor bench workflow #20742
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
777ea28 to
144bedd
Compare
144bedd to
107e1b3
Compare
582bf20 to
2d0ae0b
Compare
3c623ab to
e43b8c7
Compare
| echo "::endgroup::" | ||
| echo "::group::compare_results" | ||
| python3 ./devops/scripts/benchmarks/compare.py to_hist \ | ||
| --avg-type EWMA \ | ||
| --cutoff "$(date -u -d '7 days ago' +'%Y%m%d_%H%M%S')" \ | ||
| --name "$SAVE_NAME" \ | ||
| --compare-file "./llvm-ci-perf-results/results/${SAVE_NAME}_${SAVE_TIMESTAMP}.json" \ | ||
| --results-dir "./llvm-ci-perf-results/results/" \ | ||
| --compare-file "${{ steps.results_repo.outputs.BENCHMARK_RESULTS_REPO_PATH }}/results/${SAVE_NAME}_${SAVE_TIMESTAMP}.json" \ | ||
| --results-dir "${{ steps.results_repo.outputs.BENCHMARK_RESULTS_REPO_PATH }}/results/" \ | ||
| --regression-filter '^[a-z_]+_sycl .* CPU count' \ | ||
| --regression-filter-type 'SYCL benchmark (measured using CPU cycle count)' \ | ||
| --verbose \ | ||
| --produce-github-summary \ | ||
| ${{ inputs.dry_run == 'true' && '--dry-run' || '' }} \ | ||
| echo "::endgroup::" | ||
|
|
||
| - name: Run benchmarks integration tests | ||
| shell: bash | ||
| if: ${{ github.event_name == 'pull_request' }} |
Check failure
Code scanning / zizmor
dangerous use of environment file Error test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lukaszstolarczuk Do we need to do something about this (github security issue)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I fixed it, but I'm not sure if zizmor correctly showed me the issue.
Anyway, I will take another look at the zizmor alerts later on (I can see a few other alerts in the benchmark/action.yml file which are not displayed here - will take care of them in another PR).
e43b8c7 to
82b2f04
Compare
8eb3fe7 to
02b8752
Compare
02b8752 to
50b91ca
Compare
|
dispatch run: https://github.com/intel/llvm/actions/runs/19759131571/job/56617416122#step:19:1007 |
|
@sarnex , can you please re-review? |
|
Sorry I don't know how I missed so many pings |
sarnex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no major flags besides this one thing, in depth review left to benchmark team
| echo "::endgroup::" | ||
| echo "::group::compare_results" | ||
| python3 ./devops/scripts/benchmarks/compare.py to_hist \ | ||
| --avg-type EWMA \ | ||
| --cutoff "$(date -u -d '7 days ago' +'%Y%m%d_%H%M%S')" \ | ||
| --name "$SAVE_NAME" \ | ||
| --compare-file "./llvm-ci-perf-results/results/${SAVE_NAME}_${SAVE_TIMESTAMP}.json" \ | ||
| --results-dir "./llvm-ci-perf-results/results/" \ | ||
| --compare-file "${{ steps.results_repo.outputs.BENCHMARK_RESULTS_REPO_PATH }}/results/${SAVE_NAME}_${SAVE_TIMESTAMP}.json" \ | ||
| --results-dir "${{ steps.results_repo.outputs.BENCHMARK_RESULTS_REPO_PATH }}/results/" \ | ||
| --regression-filter '^[a-z_]+_sycl .* CPU count' \ | ||
| --regression-filter-type 'SYCL benchmark (measured using CPU cycle count)' \ | ||
| --verbose \ | ||
| --produce-github-summary \ | ||
| ${{ inputs.dry_run == 'true' && '--dry-run' || '' }} \ | ||
| echo "::endgroup::" | ||
|
|
||
| - name: Run benchmarks integration tests | ||
| shell: bash | ||
| if: ${{ github.event_name == 'pull_request' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lukaszstolarczuk Do we need to do something about this (github security issue)
8f57263 to
02ae04b
Compare
|
I started another dispatch to be sure I didn't break anything: https://github.com/intel/llvm/actions/runs/20140113209 |
| export SAVE_SUFFIX=$SAVE_SUFFIX | ||
| echo "SAVE_SUFFIX=$SAVE_SUFFIX" >> $GITHUB_ENV | ||
| export MACHINE_TYPE=$MACHINE_TYPE | ||
| echo "MACHINE_TYPE=$MACHINE_TYPE" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: GPU_TYPE would be more accurate here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| fi | ||
| - name: Cache changes and upload github summary | ||
| export LLVM_BENCHMARKS_UNIT_TESTING=1 | ||
| export COMPUTE_BENCHMARKS_BUILD_PATH=$BENCH_WORKDIR/compute-benchmarks-build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not putting these two in env section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
02ae04b to
0077156
Compare
|
@intel/llvm-gatekeepers, changes are only in benchmarks' workflow - issues in tests are not related. Please merge |
0077156 to
fc3931b
Compare
|
Failing e2e tests are unrelated, this patch contains only workflow changes. I'll merge it. |
This reverts commit 666693d.
No description provided.