llvm: make libLLVM.so instead of many .so files #12759
Conversation
Having many LLVM shared libraries makes code that links against LLVM slow because the libraries needlessly communicate with each other via extern symbols when the internal linking could have been static.
|
OK, I haven't tested this in the nixpkgs ecosystem, but I verified that these cmake options indeed produce the desired output of a single libLLVM.so whose dynamic dependencies include only system dependencies, like these: I'm currently building the package with my patch and will test if clang works successfully with these changes to the LLVM package. |
vcunat
added
0.kind: enhancement
|
Increasing the size by 0.5 GB by default is no good, as e.g. mesa drivers depend on this and they're used by quite a lot of people (all intel for example). |
|
Putting *.a into a separate output might be the best solution. |
Can you explain the connection between the increase in size and the problem with depending on the package? Also as a reminder, the option is still on the table to produce libLLVM.so and leave out the .a files. |
|
I built clang_37 with this patch, and indeed invoking clang is 8 times faster with the patch. However I'm getting some linker errors in my project when trying to depend on LLVM now - let me resolve these before merging... |
|
Also, I overestimated the addition in size. I was looking at a debug build, but the release build has these sizes:
|
|
OK, as far as I'm concerned, this patch is ready for review. It works for my use case and it builds clang successfully. Provided that the rest of the ecosystem builds, this patch is my proposal. This is a huge performance improvement for anything that relies on LLVM. |
They pull libLLVM into the runtime closure, i.e. everyone using mesa drivers will have those files. |
|
What is |
vcunat
added
the
1.severity: mass-rebuild
From CMakeLists.txt: option(LLVM_DYLIB_EXPORT_ALL "Export all symbols from libLLVM.dylib (default is C API only" OFF) |
|
Those docs are out of date. They list another option, |
|
Mesa has a problem with this: |
|
Mesa will perform better if it links against libLLVM.so instead of individual shared libraries. It will also perform better if it statically links against the .a files, but the tradeoff there is +95MB. Statically linking sounds nice to me, but if for some reason the extra file size is not acceptable, then I think it would be worth it to try to get the mesa package to work with libLLVM.so. |
|
Arch linux patches the configure file to replace the other .so files to use libLLVM.so: https://projects.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/mesa#n26 |
|
As far as I can tell, Debian builds mesa against LLVM static libraries: https://anonscm.debian.org/cgit/pkg-xorg/lib/mesa.git/tree/debian/rules |
|
I think trying to resist static linking against LLVM is an uphill battle; static libraries are the default output of LLVM and static libraries are the default input of Mesa. If nixos makes the decision to do that battle, however, then we should do it right and use libLLVM.so instead of the many other libs which kills performance. |
|
I think that battle is fought by many distros. Debian-based systems use mesa linked against |
|
I expect the main problem is that too many separate mesa libs need to link against LLVM. |
|
I've been using clang with this patch in a nix-shell for the past couple of days, and it is an order of magnitude faster than before. I understand it's tricky to fundamentally change the way LLVM is linked, but we can't pass up a performance improvement like this. We need to make this happen. |
|
Nothing really prevents us from using a differently set up llvm for mesa than for other packages. (That could also avoid the mass rebuild.) |
|
Maybe However, looking at http://hydra.nixos.org/build/30758676/contents/1, it appears that we don't actually build static libraries for llvm anymore. So moving libLLVM.so into a separate output would only save a couple dozen megabytes from the runtime closure (basically, the bin/ and include/ directories). |
|
This PR enables static libraries which makes the result rather large IIRC. (The main problem might be that the tools get statically linked.) |
|
As is, this PR enables static libraries. I personally think having the static libraries is nice, but we also have the option of not shipping the static libraries and have the package only export the single .so file. Then we have the job of making sure that every package which depends on LLVM knows how to build against the single .so instead of many. |
|
FWIW, I tested the mesa-noglu package with I looked at the output size and I'm honestly shocked at how small it is: --- a/pkgs/development/libraries/mesa/default.nix
+++ b/pkgs/development/libraries/mesa/default.nix
@@ -95,7 +95,7 @@ stdenv.mkDerivation {
"--with-egl-platforms=x11,wayland,drm"
"--enable-gallium-llvm"
- "--enable-llvm-shared-libs"
+ "--disable-llvm-shared-libs"
] ++ optional enableTextureFloats "--enable-texture-float"
++ optional grsecEnabled "--enable-glx-rts"; # slight performance degradation, enable only for grsec |
|
Hmm maybe those weren't the correct output files. Maybe these: |
|
These file sizes all seem reasonable to me. |
|
No, the sizes aren't reasonable at all. A year ago http://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.mesa_drivers.x86_64-linux#tabs-charts |
|
LLVM 3.8.0 is released: http://llvm.org/releases/3.8.0/docs/ReleaseNotes.html From the release notes:
Perhaps in the llvm_38 package we could enable this option? |
|
Yes, that sounds promising. |
vcunat's review: - let's not switch the default versions of llvm* for now - the only changes I see is adding python to clang's buildInputs and using the big so-file as discussed in #12759 (BUILD_SHARED_LIBS -> LLVM_LINK_LLVM_DYLIB) - in future it will be nice to split libLLVM into a separate output
vcunat's review: - let's not switch the default versions of llvm* for now - the only changes I see is adding python to clang's buildInputs and using the big so-file as discussed in #12759 (BUILD_SHARED_LIBS -> LLVM_LINK_LLVM_DYLIB) - in future it will be nice to split libLLVM into a separate output (cherry picked from commit f5fe051)
|
This is fixed as of llvm 3.8.0 package. |
vcunat's review: - let's not switch the default versions of llvm* for now - the only changes I see is adding python to clang's buildInputs and using the big so-file as discussed in NixOS#12759 (BUILD_SHARED_LIBS -> LLVM_LINK_LLVM_DYLIB) - in future it will be nice to split libLLVM into a separate output (cherry picked from commit f5fe051)
Having many LLVM shared libraries makes code that links against
LLVM slow because the libraries needlessly communicate with each other
via extern symbols when the internal linking could have been static.
This change enables a setting which causes all the LLVM libraries to be statically linked together, and then into a final shared library, libLLVM.so. This is the strategy used, for example, in the arch package.
My motivation for this pull request is my own compiler project which is encumbered by the way LLVM is currently linked. Resolving dynamic symbols is currently a performance bottleneck.
However, this also affects other NixOS packages that depend on LLVM, such as rustc. Try running
rustc --helpand you will find it takes almost 100ms. Now compare that tocat --helpwhich is almost instant. If you use callgrind to analyze what is taking the time, it's all in resolving the LLVM shared libs.In fact now that I'm thinking about it, and I just tested it, the same is true about clang itself. The clang compiler spends a lot of time resolving dynamic symbols to LLVM which makes compile times slower.
I have not finished testing this change, but I want to open it up for discussion while doing so.
We have the option of leaving the .a files around for those who wish to link statically against LLVM, and deleting the .a files to force dependant packages to use libLLVM.so. In the former case this patch would slightly reduce disk usage, and in the latter case this patch would increase disk usage of the LLVM package by the binary size of all the .a files, which is about 450MB. My preference and the default option that happens if we take no action is the former - leave the .a files for those who want them. This is status quo for libclang.
Previously, this nixpkg deviated from the default build options by generating .so files instead of .a files. With this patch, the nixpkg will deviate from the default build options only by additionally generating libLLVM.so alongside the .a files.
In short, I expect this change to drastically speed up any program which depends on LLVM. The only challenge will be making sure that packages which depend on LLVM still build correctly.
cc @lovek323
cc @7c6f434c
cc @viric