-
Notifications
You must be signed in to change notification settings - Fork 6
Use Preferences.jl for compile-time CPU configuration #31
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
Use Preferences.jl for compile-time CPU configuration #31
Conversation
6e754c2
to
a33a635
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
=========================================
- Coverage 16.74% 8.06% -8.69%
=========================================
Files 5 5
Lines 209 186 -23
=========================================
- Hits 35 15 -20
+ Misses 174 171 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3fdbfd3
to
a6b0ef0
Compare
Minimal change to replace runtime variables in __init__ with compile-time preferences using exact same variable names (nc, syst). Users can now set preferences to override CPU detection: - nc: number of cores - syst: system threads The __init__ function uses preferences if set, otherwise falls back to runtime detection. This eliminates runtime overhead when preferences are configured. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
a33a635
to
5c75529
Compare
Do we actually need the configurability (to set the number of threads, etc) or would it suffice to have something like |
Part 4 Loopvectorization fails downstream JuliaSIMD/LoopVectorization.jl#551.
Same two failures downstream. So let's do it. |
I don't think this should have been merged as-is. Right now if someone, e.g., uses PackageCompiler on a large machine, you can end up with |
Didn't the other form just error? This unblocks a lot of code while keeping the same assumptions it had before. Those assumptions might not be great but it's keeping to the current design of the package |
No, before the number of HW threads would be measured at run-time via the |
That case had some odd behavior though? Since the package was written with that information all static, it would effectively invalidate all code in the built binary. Also, I thought the most recent tests had showed CPUSummary would break PackageCompiler builds? I kind of think that if this made only runtime, that would almost be a different library. All things built on this (VectorizationBase, Polyester, PolyesterWeave, LoopVectorization, RecursiveFactorization, etc.) all. rely on the assumption of it being static information. Changing this to runtime information would be a major breaking change that would require rewriting all of those libraries, which is a major overhaul that I don't think anyone is up for. I think it could be reasonable for this to be a v0.3, or v1.0, though. I had debated it but ultimately thought it was non-breaking because all downstream integrations tests pass, since indeed the only thing that changed was some form of relocatability. |
What about a compromise solution that sets the value eagerly with a measurement at precompile-time, and then only invalidates in the That way:
It's still not perfect (in particular for |
Looking at this, that's actually how this code already worked before (except for the Preferences pieces) I think it would be sufficient to have:
If you're seeing invalidations in the wild from this, then that means that one of the pieces of information is not stable from pre-compile-time, which should be investigated instead of keeping it fixed and hoping for the best (I don't think it's expected that any of this changes on a given machine?) |
That wasn't trim compatible though? |
It is trim compatible if you set the preference |
Summary
This PR replaces the runtime
__init__
function configuration with compile-time preferences using Preferences.jl, as requested in the issue. This allows users to override CPU detection values at compile time rather than runtime.Changes
__init__
function to remove runtime evaluation (lines 55-69)set_cpu_preferences()
function for user conveniencex86.jl
andgeneric_topology.jl
to respect preferencesKey Benefits
@eval
in__init__
Usage Example
Testing
🤖 Generated with Claude Code