-
Notifications
You must be signed in to change notification settings - Fork 792
[Benchmarks] remove force rebuild path and unify installdir use #20388
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?
Conversation
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.
Pull Request Overview
This PR simplifies the benchmark rebuild logic by removing the --no-rebuild
option and consolidating the decision of whether to use an install directory into the GitProject
constructor. Previously, rebuild behavior was controlled through multiple mechanisms (force_rebuild
, check_install
, check_build
parameters), which was error-prone. Now, the system automatically skips rebuilds when a project's compiled artifacts exist and its version matches the expected commit hash.
Key changes:
- Removed
--no-rebuild
CLI option andoptions.rebuild
flag - Replaced
force_rebuild
parameter withuse_installdir
inGitProject
constructor - Simplified
needs_rebuild()
to check either install or build directory based onuse_installdir
- Removed
install_prefix
parameter fromconfigure()
method - CI now cleans workdir before running to ensure clean builds
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
devops/scripts/benchmarks/git_project.py | Refactored GitProject to use use_installdir flag, simplified needs_rebuild() and configure() methods |
devops/scripts/benchmarks/options.py | Removed rebuild option field |
devops/scripts/benchmarks/main.py | Removed --no-rebuild argument and related assignment |
devops/scripts/benchmarks/utils/compute_runtime.py | Updated calls to remove check_install parameter and install_prefix argument; added use_installdir=False for compute-runtime |
devops/scripts/benchmarks/benches/*.py | Updated benchmark classes to use use_installdir=False and removed install_prefix arguments |
devops/scripts/benchmarks/benches/velocity.py | Removed manual build directory cleanup logic |
devops/scripts/benchmarks/README.md | Removed documentation about --no-rebuild option |
devops/actions/run-tests/benchmark/action.yml | Added workdir cleanup before running benchmarks |
Co-authored-by: Copilot <[email protected]>
Benchmark changes tested in: https://github.com/intel/llvm/actions/runs/18593660864/job/53014846090 |
Please, run benchmarks with a broader set of suites, not only Compute Benchmarks: |
|
|
||
### Rebuild | ||
The scripts will try to reuse the files stored in `~/benchmarks_workdir/`, but the benchmarks will be rebuilt every time. | ||
To avoid that, use `--no-rebuild` option. |
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.
Actually, a comment that scripts try to reuse binaries or rebuild projects automatically would be useful.
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.
applied
Defining whether project use install dir is in one place - GitProject constructor, not in configure and needs_rebuild arguments which was error prone. No force rebuild path any more.
"--no-rebuild" option to devops/scripts/benchmarks/main.py is removed and the behavior of the script is the same as previously was with that option. This simplifies the code
Previously --no-rebuild option caused compute-benchmarks, llama , etc... dependent project to be not rebuild if already compiled in proper version (commit hash hardcoded in benchmarks scripts matches).
Now this behavior is default and the only one.
if suite is enabled by options (--sycl, --ur, etc..), compiled and its tag (e.g. compute-benchmarks tag) matches benchmarks's hardcoded one - it is not rebuild, otherwise it is rebuild.
In CI simply workdir is cleaned before running main.py, so previous behavior is preserved.
During manual work, if one wants to force a rebuild a dependent repo he/she needs to remove its binary dir from workdir.