Skip to content
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

[CI][BENCHMARK] Merge new metadata implementation + tweaks to benchmarks #17617

Merged
merged 89 commits into from
Mar 26, 2025

Conversation

ianayl
Copy link
Contributor

@ianayl ianayl commented Mar 24, 2025

This PR partially merges changes introduced in #17229. Specifically, it merges changes related to:

  • Introducing metadata in the result
  • Tweaks to the benchmarks itself: benchmarks ran, parameters used to run, what is enabled/disabled, etc
    • Enables/Disables benchmarks we are interested/no longer interested in
    • Disables non-working benchmarks
    • Bumps commit hashes used to fetch each benchmark
  • New command line arguments, such as --build-jobs and --redownload (to re-fetch the benchmarks)

Note: I am relying on this PR having its commits squashed during merge (which should be the default behavior for intel/llvm)

I did not try to clean up the commit history. Most of these changes are by Piotr anyway, I wanted to make sure he was properly credited for the changes.

Updates for current splitting effort of #17229: #17545 (comment)

ianayl and others added 30 commits February 27, 2025 14:01
This patch improves numerous aspects on how the benchmarking
results are visualized:
 - rewrites the way HTML charts are generated, using a library (Chart.js)
 that's both easier to use and more visually pleasing.
 The new HTML page also now decouples data from the HTML itself,
 leading to faster load times and the ability to fetch data
 from remote sources.
 - The markdown output now contains a failures section that
 lists all benchmarks that failed for a given run. This will be
 a helpful for developers during PR testing.
 - Benchmarks can now have description that's displayed on the page.
 - And many more minor improvements.
On PRs based on main, the scripts location is "old" and not accesible.
Pick location based on the dir existance. Step 'gather info' is in
a 'weird' location, so solve it with 2 tries to execute the script.
Co-authored-by: Piotr Balcer <[email protected]>
@ianayl ianayl temporarily deployed to WindowsCILock March 25, 2025 16:01 — with GitHub Actions Inactive
@ianayl
Copy link
Contributor Author

ianayl commented Mar 26, 2025

@intel/llvm-gatekeepers PR ready for merging, thanks!

@pbalcer
Copy link
Contributor

pbalcer commented Mar 26, 2025

@intel/llvm-gatekeepers please merge

@martygrant
Copy link
Contributor

martygrant commented Mar 26, 2025

there's a failing job, I may have saw a comment about this that it's a job to be removed so can be ignored?

@martygrant
Copy link
Contributor

found the comment - #17562 (comment) - maybe this is something we can get removed

@martygrant martygrant merged commit 8032832 into sycl Mar 26, 2025
22 of 23 checks passed
@ianayl ianayl deleted the benchmark-scripts-metadata branch March 26, 2025 14:21
@ianayl
Copy link
Contributor Author

ianayl commented Mar 26, 2025

found the comment - #17562 (comment) - maybe this is something we can get removed

Yeah... I'm not actually sure why that job still exists, I'll ask around

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants