Probe and dlopen() the correct libstdc++ #46976
Conversation
|
Also, to bikeshed the names of things a bit, I tend to prefer "verb-object" names where possible, so One thing that I'm not 100% happy about here is that we will be introducing a bit of a behavior change on Linux versus everything else. In particular, the path of our bundled |
I agree with keeping behavior consistent across all platforms, but I'm also not sure if this is an issue on other platforms. Also obviously if we want to do this on Windows it's going to look completely different. |
|
Om my machine, the path that is loaded is: |
|
path normalization is likely unnecessary, as the loader will probably capture the realpath later (a function which, notoriously, is the only one that does suffer from PATH_MAX issues, and has no posix replacement 👀) |
|
Currently, buildkite shows that it does fail to |
|
Do we have an idea of why? I can't easily tell why this should fail to dlopen system libstdc++ (and I'm not even sure this is system libstdc++ at this stage yet as I believe Julia's one still has the priority). |
|
|
@apaz-cli you can debug this with the same environment as on CI by using the For the record, there is a |
|
Alright, definite progress! Next thing is to do the moving of |
|
This PR should be ready to merge, unless there's something else to add. The previous changes were comment typos only. |
|
There is a conflict now with CSL_NEXT_GLIBCXX_VERSION, since that was recently bumped, but once you resolve that, it seems good to me. |
|
Final benchmarks, f7d4edc ( hot start: % hyperfine -r 50 "./julia --startup-file=no -e ''" "46976-mg-dlopen-libstdc++/julia --startup-file=no -e ''"
Benchmark 1: ./julia --startup-file=no -e ''
Time (mean ± σ): 114.4 ms ± 1.0 ms [User: 87.2 ms, System: 75.9 ms]
Range (min … max): 113.1 ms … 117.2 ms 50 runs
Benchmark 2: 46976-mg-dlopen-libstdc++/julia --startup-file=no -e ''
Time (mean ± σ): 114.6 ms ± 1.0 ms [User: 88.6 ms, System: 74.2 ms]
Range (min … max): 113.2 ms … 117.3 ms 50 runs
Summary
'./julia --startup-file=no -e ''' ran
1.00 ± 0.01 times faster than '46976-mg-dlopen-libstdc++/julia --startup-file=no -e '''cold start: % hyperfine --prepare 'sync; echo 3 | sudo tee /proc/sys/vm/drop_caches' -r 50 "./julia --startup-file=no -e ''" "46976-mg-dlopen-libstdc++/julia --startup-file=no -e ''"
Benchmark 1: ./julia --startup-file=no -e ''
Time (mean ± σ): 253.1 ms ± 3.9 ms [User: 141.5 ms, System: 175.6 ms]
Range (min … max): 245.4 ms … 262.2 ms 50 runs
Benchmark 2: 46976-mg-dlopen-libstdc++/julia --startup-file=no -e ''
Time (mean ± σ): 249.6 ms ± 6.7 ms [User: 134.3 ms, System: 178.0 ms]
Range (min … max): 238.3 ms … 275.8 ms 50 runs
Summary
'46976-mg-dlopen-libstdc++/julia --startup-file=no -e ''' ran
1.01 ± 0.03 times faster than './julia --startup-file=no -e '''Timings are compatible within error bars. Note that in the meantime #45582 has been merged, so the fact timings are consistent is good because now this probe should be no-op. Thanks a lot @apaz-cli for this work! 🚀 |
vtjnash
added
backport 1.8
* Probe if system libstdc++ is newer than ours If the system libstdc++ is detected to be newer, load it. Otherwise, load the one that we ship. This improves compatibility with external shared libraries that the user might have on their system. Fixes #34276 Co-authored-by: Jameson Nash <[email protected]> Co-authored-by: Elliot Saba <[email protected]> * Addressed review comments. * Change error handling in wrapper functions Co-authored-by: Jameson Nash <[email protected]> * Call write_wrapper three times instead of snprintf Co-authored-by: Jameson Nash <[email protected]> * Apply suggestions from code review Co-authored-by: Jameson Nash <[email protected]> * Update cli/loader_lib.c Co-authored-by: Jameson Nash <[email protected]> * Reordered reading and waiting to avoid a deadlock. * Fixed obvious issues. * Only load libstdc++ preemptively on linux. * Update cli/loader_lib.c Co-authored-by: Jameson Nash <[email protected]> * Update cli/loader_lib.c Co-authored-by: Jameson Nash <[email protected]> * Specified path to bundled libstdc++ on the command line. * Removed whitespace. * Update cli/Makefile Co-authored-by: Jameson Nash <[email protected]> * Handled make install stringreplace. * Correctly quoted stringreplace. * Added -Wl,--enable-new-dtags to prevent DT_RPATH for transitive dependencies * Updated news entry. * Added comment about environment variable. * patched rpath for libgfortran and libLLVM. * Added explaination to Make.inc * Removed trailing space * Removed patchelf for libgfortran, now that BB has been fixed. * Fixed typos and comments Co-authored-by: Max Horn <[email protected]> Co-authored-by: Mosè Giordano <[email protected]> Co-authored-by: Jameson Nash <[email protected]> Co-authored-by: Elliot Saba <[email protected]> Co-authored-by: Max Horn <[email protected]> (cherry picked from commit eb708d6)
|
This has broken build on Linux with |
After adding `libstdc++` probing into the Julia loader [0], we originally made the assumption that the `libstdc++` that is shipped with `julia` would always be co-located with `libjulia.so` [1]. This is not the case when building with `USE_SYSTEM_CSL=1`, however, where we sequester system libraries in `usr/lib/julia`, even at build-time. The path to `libstdc++.so` has already been getting altered when moving from build-time to install time via `stringreplace` [2], but after further thought, I decided that it would be better to just use the pre-existing `LOADER_*_DEP_LIBS` mechanism to communicate to the loader what the correct relative path to `libstdc++.so` is. This also allows the single `stringreplace` to update all of our "special" library paths. [0] #46976 [1] https://github.com/JuliaLang/julia/pull/46976/files#diff-8c5c98f26f3f7aac8905a1074c5bec11a57e9b9c7c556791deac5a3b27cc096fR379 [2] https://github.com/JuliaLang/julia/blob/master/Makefile#L430
After adding `libstdc++` probing into the Julia loader [0], we originally made the assumption that the `libstdc++` that is shipped with `julia` would always be co-located with `libjulia.so` [1]. This is not the case when building with `USE_SYSTEM_CSL=1`, however, where we sequester system libraries in `usr/lib/julia`, even at build-time. The path to `libstdc++.so` has already been getting altered when moving from build-time to install time via `stringreplace` [2], but after further thought, I decided that it would be better to just use the pre-existing `LOADER_*_DEP_LIBS` mechanism to communicate to the loader what the correct relative path to `libstdc++.so` is. This also allows the single `stringreplace` to update all of our "special" library paths. [0] #46976 [1] https://github.com/JuliaLang/julia/pull/46976/files#diff-8c5c98f26f3f7aac8905a1074c5bec11a57e9b9c7c556791deac5a3b27cc096fR379 [2] https://github.com/JuliaLang/julia/blob/master/Makefile#L430
After adding `libstdc++` probing into the Julia loader [0], we originally made the assumption that the `libstdc++` that is shipped with `julia` would always be co-located with `libjulia.so` [1]. This is not the case when building with `USE_SYSTEM_CSL=1`, however, where we sequester system libraries in `usr/lib/julia`, even at build-time. The path to `libstdc++.so` has already been getting altered when moving from build-time to install time via `stringreplace` [2], but after further thought, I decided that it would be better to just use the pre-existing `LOADER_*_DEP_LIBS` mechanism to communicate to the loader what the correct relative path to `libstdc++.so` is. This also allows the single `stringreplace` to update all of our "special" library paths. [0] #46976 [1] https://github.com/JuliaLang/julia/pull/46976/files#diff-8c5c98f26f3f7aac8905a1074c5bec11a57e9b9c7c556791deac5a3b27cc096fR379 [2] https://github.com/JuliaLang/julia/blob/master/Makefile#L430
After adding `libstdc++` probing into the Julia loader [0], we originally made the assumption that the `libstdc++` that is shipped with `julia` would always be co-located with `libjulia.so` [1]. This is not the case when building with `USE_SYSTEM_CSL=1`, however, where we sequester system libraries in `usr/lib/julia`, even at build-time. The path to `libstdc++.so` has already been getting altered when moving from build-time to install time via `stringreplace` [2], but after further thought, I decided that it would be better to just use the pre-existing `LOADER_*_DEP_LIBS` mechanism to communicate to the loader what the correct relative path to `libstdc++.so` is. This also allows the single `stringreplace` to update all of our "special" library paths. [0] #46976 [1] https://github.com/JuliaLang/julia/pull/46976/files#diff-8c5c98f26f3f7aac8905a1074c5bec11a57e9b9c7c556791deac5a3b27cc096fR379 [2] https://github.com/JuliaLang/julia/blob/master/Makefile#L430 (cherry picked from commit fb97c82)
Fixes JuliaCI/julia-buildkite#205
Fixes #34276
Fixes JuliaGL/GLFW.jl#198
Initial implementation by @giordano
To opt-out of the fix, set the
JULIA_PROBELIBSTDCXXenvironment variable to0; e.g.export JULIA_PROBELIBSTDCXX=0.TODO:
libstdcxx.so.6that we ship off rpathmake installLIBSTDCXXPROBEa good name for the#defineandJULIA_PROBELIBSTDCXXa good name for the environment variable?CSL_NEXT_GLIBCXX_VERSIONfor the symbol to extract and check for compatibility.