Repository navigation
[cmake] Require explicit CUDA feature selection - #23903
guitargeek merged 2 commits into
Conversation
CUDA compiler discovery starts a separate CMake configuration even when neither CUDA feature is requested. This can add substantial overhead on slow I/O and NFS-backed filesystems. Only discover CUDA for explicit requests or when all needs availability information to select defaults. Preserve explicit OFF settings and gminimal defaults, and cover these combinations with regression tests. Assisted-by: Codex:GPT-6
|
@guitargeek btw, with the plan of externalising SOFIE, do we still need the tmva-gpu option? |
|
The |
Test Results 21 files 21 suites 3d 10h 23m 19s ⏱️ For more details on these failures, see this check. Results for commit 33f50fc. ♻️ This comment has been updated with latest results. |
|
Thanks. Agreed then! |
guitargeek
left a comment
guitargeek
left a comment
There was a problem hiding this comment.
Thank you very much for bringing this up! I'd just suggest to go one step further, simplifying the logic by not considering cuda and tmva-gpu as "major options". The all build flag doesn't enable all options, and it's mostly up to our judgment what we consider as "major".
What do you think? Would that suggestion also work for you?
Also, would it be fine to remove the unit test again? AI agents like to test everything, but I would prefer to not start entering the rabbit hole of testing CMake configuration paths from now on...
Keep CUDA features opt-in even with all enabled, including when CUDA compiler information is already cached. Simplify compiler discovery and remove the configuration test suite as requested in review. Validate option combinations using a temporary external harness. Assisted-by: Codex:GPT-6
guitargeek
left a comment
guitargeek
left a comment
There was a problem hiding this comment.
LGTM! Thank you very much
|
@jmcarcell I assume you need a backport to 6.42. Also older ROT releases? |
|
It's ok if it's only in master. |
jmcarcell
changed the title
|
Yes, that's why I prefer to not backport it to the existing patch release branches. 6.42 is so far unreleased (there is only a release candidate), so I think that is okay |
|
/backport to 6.42 |
|
Preparing to backport PR #23903 to branch 6.40 requested by guitargeek |
|
Preparing to backport PR #23903 to branch 6.42 requested by guitargeek |
|
This PR has been backported to branch 6.40: #23946 |
|
This PR has been backported to branch 6.42: #23947 |
This Pull request:
Require explicit enablement of CUDA features, including in builds configured with
all=ON.Changes or fixes:
ROOT currently probes for a CUDA compiler on every configure and, with
all=ON, automatically enablescudaandtmva-gpuwhen a compiler is found. Compiler discovery starts a separate CMake configuration and accesses compiler and toolkit files, adding avoidable configuration time in slow I/O environments and on NFS-backed filesystems.Only probe for and initialize CUDA when
cudaortmva-gpuis enabled. Both options remain OFF by default underall, even when CUDA compiler information is already cached. For example,all=ON cuda=ONenablescudawithout automatically enablingtmva-gpu.Explicit settings, including existing cached ON values, continue to take precedence over defaults. CUDA discovery and initialization for explicitly enabled features retain their existing behavior.
Checklist:
No documentation changes are needed. No associated issue.