Remove deps folder and check AES-NI in __init__().
#11
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11 +/- ##
==========================================
- Coverage 98.54% 98.12% -0.42%
==========================================
Files 7 8 +1
Lines 822 853 +31
==========================================
+ Hits 810 837 +27
- Misses 12 16 +4
Continue to review full report at Codecov.
|
|
Looks good now... |
|
This broke downstream compilation of packages: See JuliaGPU/CUDA.jl#1422 |
|
Ah I missed this comment! I'll look into it later |
| function __init__() | ||
|
|
||
| R123_USE_AESNI[] = try | ||
| cmd = Base.julia_cmd() |
There was a problem hiding this comment.
__init__ is for simple initialization; spawning a Julia process is not that and should not happen there. Either do this globally, and embed the use of AES-NI into the precompilation image (which should be safe, as that will not change across sessions) or make accesses to related functions lazy and have them initialize the bool upon first use.
There was a problem hiding this comment.
What do you mean by doing it globally? We used to do this in deps/build.jl but then decided that checking it every time the package is loaded may be better.
There was a problem hiding this comment.
Once, globally. const use_aesni = .... That'll be embedded in your precompiled package and not be evaluated every time you import the package. It's still better than deps/build.jl, which will only fire once when you install the package, since precompilation images are (should be) regenerated once the environment changes.
But again, for full flexibility, use a dynamic getter (a function that returns whether to use AES-NI, computes that lazily, and caches it per session). Look at how Flux.jl decides whether to use GPU support or not, for inspiration.
There was a problem hiding this comment.
Thank you so much. This is really helpful! I'm gonna have a look when I have time.
| end | ||
|
|
||
| if R123_USE_AESNI[] | ||
| @eval using ._AESNIModule |
There was a problem hiding this comment.
This is not allowed, and triggers JuliaGPU/CUDA.jl#1422.
There was a problem hiding this comment.
I have seen that. I realized @eval may not be necessary. Will look into this later..
There was a problem hiding this comment.
You cannot do a using at non-toplevel scope. So this will never work from __init__.
There was a problem hiding this comment.
Oh I see... Is there a way to lazy load this? Like what Requires.jl does
There was a problem hiding this comment.
It's possible, one part of the trick is not to do so while precompiling, https://github.com/JuliaPackaging/Requires.jl/blob/542a641b633fa38a5950c2b1dbb90861967c7015/src/require.jl#L97-L105.
No description provided.