-
Notifications
You must be signed in to change notification settings - Fork 143
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
Make HDF5.jl compatible with profilers that use LD_PRELOAD #791
base: master
Are you sure you want to change the base?
Conversation
and use references for some constants
@@ -38,7 +38,7 @@ else | |||
if libhdf5_size != filesize(Libdl.dlpath(libhdf5)) | |||
error("HDF5 library has changed, re-run Pkg.build(\\\"HDF5\\\")") | |||
end | |||
if libversion < v"1.10.4" | |||
if libversion[] < v"1.10.4" |
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.
libversion
is now set in HDF5.__init__
so it needs to be a Ref.
@@ -709,7 +709,7 @@ dspace_scal = HDF5.Dataspace(HDF5.h5s_create(HDF5.H5S_SCALAR)) | |||
dspace_norm = dataspace((100, 4)) | |||
dspace_maxd = dataspace((100, 4), max_dims = (256, 4)) | |||
dspace_slab = HDF5.hyperslab(dataspace((100, 4)), 1:20:100, 1:4) | |||
if HDF5.libversion ≥ v"1.10.7" | |||
if HDF5.h5_get_libversion() ≥ v"1.10.7" |
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.
Call the function instead of using the const global (to do the latter, it would need to be HDF5.libversion[]
since it's a Ref).
Thanks for the PR! In particular, I don't understand why exactly:
Perhaps a tool like Cassete.jl could be used to inject code transformations here. |
I agree this isn't a small change. What Darshan and other applications do is inject their own "instrumenting" code into your code by using Unfortunately, the canonical use of I can understand your reluctance to make a fundamental change like this. But Darshan is really useful and I and others want to use it to profile HDF5 I/O. Having this capability with Julia would be nice. I hadn't thought to try transformations with |
There are several real failures in CI to take a look at. |
JLL packages already have way to allow you to point the library to something else, without having to write some custom code to do the same thing: https://docs.binarybuilder.org/dev/jll/#Non-dev'ed-JLL-packages. Would it work for you? I don't see the difference between using |
No, I don't think that mechanism will work. Perhaps take a look at JuliaParallel/MPI.jl#451 . That doesn't really help HDF5.jl, since you have bare If you want to make HDF5.jl compatible with Darshan (and that's up to you, of course) then I see two choices...
In the meantime, I have my private version of HDF5.jl which I'm using to profile i/o in my application. I'm meeting with the Darshan developers next week to understand the profiling output better. I'll try to write up what I've learned so you can see if this is useful or not. |
I'm not necessarily opposed to making these changes, but:
That being said, I think some of the minor items like using (After writing all of the above, I finally looked at your MPI.jl link — isn't the |
I agree that a larger discussion is needed. I submitted https://discourse.julialang.org/t/discussion-of-ccall-function-library/53887 . Let's see what happens. |
The discussion on discourse is indeed very insightful. Given the feedback in the thread it does sound like the course of action is to incorporate the necessary changes in Base to support this use case. |
See MPI.jl PR#450. Use of
ccall( (func, lib), ...)
is not compatible with applications that useLD_PRELOAD
to inject their own code for profiling and tracing. Darshan is an example of an application that does this for MPI and HDF5 I/O profiling.This PR has changes to explicitly load the
libhdf5
andlibhdf5_hl
shared objects withLibdl.dlopen
inHDF5.__init__
. Mostccall
statements are changed to not specify the HDF5 library (e.g. I changed the generator code).I tested on NERSC Cori Haswell nodes. HDF5 tests pass. I was able to get Darshan to work with this PR.
Happy New Year too!