-
Notifications
You must be signed in to change notification settings - Fork 792
[CI] Remove dependency on bot user for uploading benchmark CI results #20333
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
base: sycl
Are you sure you want to change the base?
Changes from all commits
df8990c
7ce4420
866b64e
b518e18
2109b73
6e73cf7
e20975f
3bea805
418bd1d
1539867
8e1f1b8
66c4b5c
baf64ef
ac56a43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,9 @@ runs: | |
python3 ./devops/scripts/benchmarks/presets.py query "$PRESET" | ||
[ "$?" -ne 0 ] && exit 1 # Stop workflow if invalid preset | ||
echo "PRESET=$PRESET" >> $GITHUB_ENV | ||
|
||
# Set branch containing benchmark CI results: | ||
echo "BENCHMARK_RESULTS_BRANCH=sycl-benchmark-ci-results" >> $GITHUB_ENV | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps just set it as an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there another way to set an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way you did it I usually use when I want to dynamically set something within the workflow. If more of a static var is required, I usually use:
e.g. in UMF workflow: it will work in both "job" and "global" scope There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this defining the env variables for a single step only? i.e. variables defined this way do not propagate across steps I use the variable again in actions/checkout call here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as I mentioned, you can set it up in "job" and/or "global" scope - see my example above set up for global scope I think, setting it up for step scope is only possible if you use |
||
- name: Compute CPU core range to run benchmarks on | ||
shell: bash | ||
run: | | ||
|
@@ -134,9 +137,10 @@ runs: | |
|
||
cd - | ||
- name: Checkout results repo | ||
shell: bash | ||
run: | | ||
git clone -b unify-ci https://github.com/intel/llvm-ci-perf-results | ||
uses: actions/checkout@v5 | ||
with: | ||
ref: ${{ env.BENCHMARK_RESULTS_BRANCH }} | ||
path: llvm-ci-perf-results | ||
- name: Run compute-benchmarks | ||
env: | ||
# Need to append "_<device>_<backend>" to save name in order to follow | ||
|
@@ -237,9 +241,8 @@ runs: | |
shell: bash | ||
run: | | ||
cd "./llvm-ci-perf-results" | ||
git config user.name "SYCL Benchmarking Bot" | ||
git config user.email "[email protected]" | ||
results_branch="unify-ci" | ||
git config user.name "github-actions[bot]" | ||
git config user.email "github-actions[bot]@users.noreply.github.com" | ||
|
||
if git diff --quiet && git diff --cached --quiet; then | ||
echo "No new results added, skipping push." | ||
|
@@ -252,7 +255,7 @@ runs: | |
git commit -m "[GHA] Upload compute-benchmarks results from https://github.com/intel/llvm/actions/runs/${{ github.run_id }}" | ||
results_file="$(git diff HEAD~1 --name-only -- results/ | head -n 1)" | ||
|
||
if git push "https://[email protected]/intel/llvm-ci-perf-results.git" "$results_branch"; then | ||
if git push; then | ||
ianayl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
echo "Push succeeded" | ||
break | ||
fi | ||
|
@@ -262,8 +265,8 @@ runs: | |
cached_result="$(mktemp -d)/$(basename $results_file)" | ||
mv "$results_file" "$cached_result" | ||
|
||
git reset --hard "origin/$results_branch" | ||
git pull origin "$results_branch" | ||
git reset --hard "origin/$BENCHMARK_RESULTS_BRANCH" | ||
git pull | ||
|
||
mv "$cached_result" "$results_file" | ||
fi | ||
|
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.
my note on this change - while it seems convenient, I can see on my PC, that repo
intel/llvm-ci-perf-results
is ~600MB - this will be extra MBs added tointel/llvm
repo, which is already quite big - just to considerUh oh!
There was an error while loading. Please reload this page.
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.
@ianayl would it be possible to compress the benchmarking data before pushing it to intel/llvm ?
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.
In theory yes, but only the archived data. I think we'll need to have a conversation on this: it might even be reasonable to delete horribly outdated data.
We could also try moving archived data to intel/llvm-ci-perf-results, but alas that'll mean we need to keep an updated bot user token again for the repository.
Do github repositories have a max storage quota?
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.
https://docs.github.com/en/repositories/creating-and-managing-repositories/repository-limits#repository-size
I think we should have that conversation before merging this PR. Currently, the size of
intel/llvm-ci-perf-results
is ~600MB, but, if unchecked the size will increase further.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.
Compressing
results
files gives ~190MB out of 320M, so I think it is better to just remove outdated data. We have data starting at March'25.How many months of data do wee need to keep. 6? In this case we will have data staring from 15th April today. The rest will be in git history, hopefully well compressed.
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.
If we go this way, we can archive e.g. once every second week, or something, and make a PR instead of pushing