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

Consider stripping uv release binaries #5683

Closed
edmorley opened this issue Aug 1, 2024 · 12 comments · Fixed by #5745
Closed

Consider stripping uv release binaries #5683

edmorley opened this issue Aug 1, 2024 · 12 comments · Fixed by #5745
Labels
releases Related to building and distributing release artifacts of uv

Comments

@edmorley
Copy link
Contributor

edmorley commented Aug 1, 2024

Hi 👋

Thank you for a great project!

I noticed that the uv release binaries (as distributed via GitHub releases) are currently quite large.

For example:

$ curl -sSfLO https://github.com/astral-sh/uv/releases/download/0.2.32/uv-aarch64-unknown-linux-gnu.tar.gz
$ tar -zx --strip-components=1 -f uv-aarch64-unknown-linux-gnu.tar.gz
$ du -ha uv uv-*
35M	uv
14M	uv-aarch64-unknown-linux-gnu.tar.gz

To put the size in perspective, 14 MB compressed and 35 MB uncompressed is very close to the size of our entire Python 3.12 install archive, which has an archive size of 11MB and uncompressed install size of 41 MB (and this is even with that archive intentionally bundling .pycs for the stdlib). For local installs the size doesn't really matter, however, in CI/CD/OCI image contexts cold caches can be prevalent and smaller OCI images are preferred (for end user UX it's not always desirable to exclude the package manager from the final run-image) etc, so large tool sizes are more noticeable.

uv's focus should absolutely be speed+compatibility+features over binary size, so I'm not suggesting there should be any user-facing compromises made to reduce binary size - but it seems there might be some low hanging fruit to reduce the size?

For example, stripping reduces the binary size in the example above by 25%:

$ strip uv
$ du -ha uv
26M	uv

And Cargo now supports stripping natively in a cross-platform way:
https://doc.rust-lang.org/cargo/reference/profiles.html#strip

[profile.release]
strip = true

I see this has been enabled already for the uv-trampoline crate:

# Automatically strip symbols from the binary.
strip = true

...but not any of the others (in the release profile at least):
https://github.com/search?q=repo%3Aastral-sh%2Fuv+strip+language%3ATOML&type=code&l=TOML

@zanieb zanieb added the releases Related to building and distributing release artifacts of uv label Aug 1, 2024
@charliermarsh
Copy link
Member

I thought we were stripping binaries via strip = true in pyproject.toml but the result above suggests we aren't, will take a look...

@konstin
Copy link
Member

konstin commented Aug 1, 2024

I've checked locally and

RUST_LOG=debug uvx maturin build --release --locked --out dist --features self-update --target x86_64-unknown-linux-gnu

is correctly setting -C link-arg=-s and producing stripped binaries. @messense Any ideas what the maturin action is doing different that it could be ignoring the tool.maturin.strip = true that we set in pyproject.toml, while it works locally?

@messense
Copy link
Contributor

messense commented Aug 2, 2024

No idea, can you try run maturin build with --debug on CI?

@konstin
Copy link
Member

konstin commented Aug 2, 2024

CI also says https://github.com/astral-sh/uv/actions/runs/10213117825/job/28257883122:

DEBUG build_wheels: maturin::compile: Running env -u CARGO "cargo" "rustc" "--features" "self-update" "--target" "x86_64-unknown-linux-gnu" "--message-format" "json-render-diagnostics" "--locked" "--manifest-path" "/home/runner/work/uv/uv/crates/uv/Cargo.toml" "--release" "--bin" "uv" "--" "-C" "link-arg=-s"

So the right arg is there. Could it be it's because we're not using mold in CI?

@messense
Copy link
Contributor

messense commented Aug 2, 2024

Have you tried comparing build artifact size with and without strip = true? I suspect that it's just that the Rust strip = true isn't as aggressive as the strip command.

@konstin
Copy link
Member

konstin commented Aug 2, 2024

It's indeed the difference between using the standard linker and mold. If i deactivate mold locally i get the same 37MB semi-stripped binary with strip = true that we see on CI. (Without strip = true, the binary is 40MB, properly stripped we are at 31MB/30MB).

So i think we have to configure mold for the linux build? strip doesn't seem to support stripping cross-compiled binaries, so we can't use it for the cross builds.

@messense
Copy link
Contributor

messense commented Aug 2, 2024

llvm-strip might support stripping cross-compiled binaries.

@edmorley
Copy link
Contributor Author

edmorley commented Aug 2, 2024

I'm curious, why is Maturin passing manual linker args via "-C" "link-arg=-s" rather than the rustc option-C strip={none,debuginfo,symbols} mentioned here?
https://doc.rust-lang.org/rustc/codegen-options/index.html#strip

Did the Maturin strip feature just predate rust-lang/rust#72110?

@konstin
Copy link
Member

konstin commented Aug 2, 2024

Yeah, the maturin option is older, we could change maturin to use the new option.

@edmorley
Copy link
Contributor Author

edmorley commented Aug 2, 2024

So i think we have to configure mold for the linux build? strip doesn't seem to support stripping cross-compiled binaries, so we can't use it for the cross builds.

I just checked one of our Rust project's cross-compiled binaries (aarch64-unknown-linux-musl-gcc cross-compiled from x86_64 Linux host using https://github.com/messense/homebrew-macos-cross-toolchains and Cargo's strip = true) and it's fully stripped, so it does seem like it is possible with ld?

@konstin
Copy link
Member

konstin commented Aug 3, 2024

Reported upstream: rust-lang/cargo#14346

konstin added a commit that referenced this issue Aug 3, 2024
konstin added a commit that referenced this issue Aug 3, 2024
Decreases linux release wheel size by 2MB.

See rust-lang/cargo#14346

Fixes #5683
@edmorley
Copy link
Contributor Author

edmorley commented Aug 3, 2024

Reported upstream: rust-lang/cargo#14346

Thank you!

I took a quick look and this seems to be a regression from rust-lang/cargo#13257 in Rust 1.77.0.

As a workaround in the meantime (and to avoid individual projects having to adjust their own Cargo.toml files), Maturin could set CARGO_PROFILE_RELEASE_STRIP=true rather than passing -C link-arg=-s or -C strip=symbols.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
releases Related to building and distributing release artifacts of uv
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants