Skip to content
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

multiple-outputs.sh: Install static libraries into $static->$dev->$out #229758

Draft
wants to merge 3 commits into
base: staging
Choose a base branch
from

Conversation

Luabee
Copy link

@Luabee Luabee commented May 3, 2023

Description of changes

During preFixup, static libraries (lib/*.a) are now installed to the $static output if it exists, or the $dev output if that exists, or just to $out as usual.

This will make it easier to prevent static libs making it into system closures.

Related issues:
#164141
#224533

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/cuda-team-roadmap-and-call-for-sponsors/29495/1

@ConnorBaker
Copy link
Contributor

Hi @Luabee -- the CUDA maintainer team sees this as a very useful feature for automatically splitting out static libraries from CUDA redistributable. A number of the redistributables we use include static and dynamic libraries and approach a gigabyte in size. Being able to split out static copies of the library would allow us to shave hundreds of megabytes!

What are your thoughts on the status of this PR? Do you see additional places for testing or improvement? I'd love to help if I can!

@Luabee
Copy link
Author

Luabee commented Jun 29, 2023

Hi @Luabee -- the CUDA maintainer team sees this as a very useful feature for automatically splitting out static libraries from CUDA redistributable. A number of the redistributables we use include static and dynamic libraries and approach a gigabyte in size. Being able to split out static copies of the library would allow us to shave hundreds of megabytes!

What are your thoughts on the status of this PR? Do you see additional places for testing or improvement? I'd love to help if I can!

At this point all the functional changes are in place. I'm just blocked on a confusing build error in an instance of gcc which I'm not familiar with. x86_64-unknown-linux-musl-stage-final-gcc-12.2.0.drv Is failing to build with a strange error related to libatomic? See here:

       last 10 log lines:
       > checking for __atomic_fetch_op for size 2... yes
       > checking for __atomic_fetch_op for size 4... yes
       > checking for __atomic_fetch_op for size 8... yes
       > checking for __atomic_fetch_op for size 16... no
       > checking whether byte ordering is bigendian... no
       > checking for the word size... 8
       > configure: error: Pthreads are required to build libatomic
       > make[1]: *** [Makefile:17005: configure-target-libatomic] Error 1
       > make[1]: Leaving directory '/build/build'
       > make: *** [Makefile:1034: all] Error 2
       For full logs, run 'nix log /nix/store/j5dc89g78iw9p1la31i6wqv08ls5pryy-x86_64-unknown-linux-musl-stage-final-gcc-12.2.0.drv'.
error: 1 dependencies of derivation '/nix/store/42hik4yb79bxlh8ddsnz3aavs84qpp80-x86_64-unknown-linux-musl-stage-final-gcc-wrapper-12.2.0.drv' failed to build
error: 1 dependencies of derivation '/nix/store/2imh1444hs2yjy7izyr9fvivp2j0vz0b-stdenv-linux.drv' failed to build
error: 1 dependencies of derivation '/nix/store/i4hf20pisp5vv3vvxdinl7k734211w7r-busybox-static-x86_64-unknown-linux-musl-1.36.0.drv' failed to build```

@aaronmondal
Copy link
Contributor

aaronmondal commented Sep 24, 2023

I believe this is a highly desirable change. I have some nix-built container images that require JAX, TensorFlow and Torch all in one image. The increased size from the cuda libraries doesn't just add to the image size, but in my case more painfully to the single-threaded compression time for the images/layers.

This PR would not only reduce the size of these kinds of images by several gigabytes, but also cut down on compression time.

One small issue I could imagine popping up after this is that some compilers require certain static archives in their default configuration to "run". So we might end up with some cuda/sycl/hip/c++ compiler packages no longer working out of the box. Fixing such issues (if they even occur at all) seems rather simple though since in those rare cases the relevant archives can just be added back to the package explicitly.

@Luabee
Copy link
Author

Luabee commented Sep 28, 2023

PSA: I won't be able to work on this at least through the rest of the year, so someone else would need to take it up if they want it merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants