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

Lightweight Flake #7

Merged
merged 3 commits into from
Aug 13, 2024
Merged

Lightweight Flake #7

merged 3 commits into from
Aug 13, 2024

Conversation

blitz
Copy link
Contributor

@blitz blitz commented Jul 29, 2024

This PR intends to make the flake more lightweight. 🪶

  • It removes rust-overlay because the default Rust toolchain is good enough and rust-overlay is large.
  • It removes LTO (and other) release build microoptimization to reduce build times.

Please take a look at the commit messages for more information.

@blitz
Copy link
Contributor Author

blitz commented Jul 29, 2024

Mmh. It seems I broke MacOS. Is there a good way to test it?

[profile.release]
codegen-units = 1
opt-level = "s"
lto = true
Copy link
Owner

@phip1611 phip1611 Aug 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing LTO from the build options cuts build times in half for no apparent downside.

Here are my measurements:

  • 1.82.0-nightly + existing codegen options : 20,91s
  • 1.82.0-nightly + default codegen options : 11,39s
  • 1.80.1 (stable) + existing codegen options: 20,77s
  • 1.80.1 (stable) + default codegen options : 11.85s

I agree that there is a benefit. Further, I verified that opt-level = "s" only saves 300K, although not impacting build times. As the micro optimization really isn't that important and it apparently solves problems for you, I'm fine with the changes.

Copy link
Owner

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

@phip1611 phip1611 enabled auto-merge August 13, 2024 09:35
@phip1611 phip1611 disabled auto-merge August 13, 2024 09:35
blitz added 3 commits August 13, 2024 11:35
The Rust overlay is very large, because it carries information about
all possible Rust toolchain versions. This is sometimes useful, but
this tool builds fine with the Rust version in Nixpkgs.

This change reduces the stuff that users have to download quite a bit.
Not only is there now rust-overlay tarball to download, you also use
the default Rust toolchain, which is probably already somewhere on
disk!
There is no need to micro-optimize the binary. Removing LTO from the
build options cuts build times in half for no apparent downside.

I've also removed the strippping, because the Cargo manual advices
against it for proper backtraces.
The MacOS build fails with:

> ++ command cargo build --release --message-format json-render-diagnostics --locked
>    Compiling gitlab-timelogs v0.2.2 (/private/tmp/nix-build-gitlab-timelogs-0.2.2.drv-0/8bm81sjfqmsk7lclw7pnddrxv785aaj3-source)
> error: linking with `cc` failed: exit status: 1

Fix by manually adding iconv as a dependency.

Other people also have encountered this:
nix-community/home-manager#3482
@phip1611 phip1611 enabled auto-merge August 13, 2024 09:35
@phip1611 phip1611 merged commit 41c7528 into phip1611:main Aug 13, 2024
13 checks passed
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.

2 participants