-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
packaging: use release builds and LTO by default #12097
Conversation
Won't this slow down debug builds? |
In which sense? Speed of compilation, speed of execution, or something else? I guess it could slow down compilation depending on how costly the level three optimizations are. I don’t know that LTO is a bad idea, generally. I could refactor this to conditionally set optimization level to three when it isn’t a debug build, similar to what I removed in the refactor. I’d also really appreciate any pointers you can give on reliably benchmarking Nix! |
This should be 100% disabled for debug builds, optimization can make chabnes to the code which make debugging really hard, inlining, the infamous "where did my code go? right it was optimized away/unrolled" |
Should debug builds be built without any optimization? Because that’s not what happens currently :/ |
I'm not part of the Nix dev team, but in my experience having all optimizations off greatly helps staring at code in gdb/lldb. Since the machine code is closer to what's actually written the source code |
Yes for debug builds we like less optimization both because it means faster builds, and because it means the debug symbols are easier to understand / closer to the code. |
@Ericson2314 so should debug builds happen without any optimization, or with optimization level two (which is what happens prior to this PR)? |
I think without any optimization, right @edolstra? We could have the full nix builds instead specify more optimization with |
We should use |
c94a74c
to
50aca93
Compare
Updated and force-pushed. Removed use of LTO is enabled when the build type is Added links to relevant references describing Meson build types and the default build type as set by Nixpkgs' setup hook for Meson. |
Updated with information about how I'm approaching "benchmarking." |
LTO on Darwin was broken until fairly recently, so that explains that. |
@ConnorBaker thanks for your work, I like the looks of this a lot now. Yeah better do that macOS disable for now (unless you know how to fix) but then let's merge it! |
Fixing it would require a bump to our pin of Nixpkgs — is it safe to say that would need to be a separate PR with more testing? |
@ConnorBaker bump to a newer 24.11 or something else? |
50aca93
to
359a084
Compare
I rebased and force-pushed; I think the locked version master is using for |
Updated the benchmarks; performance gain on my i9-13900k is basically unchanged (still 5-10%) and on macOS I'm seeing a 1-2% gain. @Ericson2314 I'm happy with this now and CI is green; anything else we need to do? |
@Radvendii I remember we had discussed a benchmark suite for Nix at some point as a way to establish a baseline and collect metrics; you may be interested in what I in the PR description. It's super gross, but that sort of functionality would be really, really helpful (and avoid people trying to roll their own suites for performance improving PRs)! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you!
Motivation
Let Meson handle optimization arguments for us.
Removed use of
debug
andoptimization
explicitly in project defaults -- these should be handled by Meson'sbuildtype
, not us.LTO is enabled when the build type is
release
orminsize
and disabled otherwise.Added links to relevant references describing Meson build types and the default build type as set by Nixpkgs' setup hook for Meson.
Testing
I'm not sure what the state-of-the-art is for benchmarking Nix. Any pointers?
EDIT: I settled on gross rigging around
nix-functional-tests
to try to get a sense of the performance impact.Step 1: Apply this patch so Meson doesn't randomly perturb malloc and runs tests sequentially: ConnorBaker@567a2b1.
I've done that in the two branches I use for benchmarking: https://github.com/ConnorBaker/nix/tree/feat/det-bench for the baseline and https://github.com/ConnorBaker/nix/tree/feat/meson-O3-LTO-det-bench for this PR.
Of course, re-using the functional test suite as "micro-benchmarks" comes with its own problems...
Step 2: Decide on a tool for comparing benchmark numbers.
I decided on
benchstat
because it has the functionality I'm looking for, it is available in Nixpkgs, and the format for input data is relatively close to what we get as output from Meson (https://go.googlesource.com/proposal/+/master/design/14313-benchmark-format.md).Step 3: Generate and transform the data.
Here's an AWK script which matches on the lines of the build output of
nix-functional-tests
which include the results of each script and transforms them into something whichbenchstat
can work with. I named itnix-functional-tests-to-gotest-format.awk
:And here's a bash script which run the builds over and over, transforming the build logs and collecting them in output files. I named it
nix-functional-tests-gotest-bench.sh
:Step 4: Summarize the data.
I'm seeing (if I'm reading this right) roughly 5-10% improvements across most of the functional tests. This is on a NixOS 25.05 (
nixos-unstable
) system with an i9-13900k andbenchstat nix-functional-tests-old.txt nix-functional-tests-new.txt
:On my MacBook Pro (16-inch Nov 2023, Apple M3 Max, macOS Version 15.3 Beta (24D5034f)) I see a much smaller performance improvement of about 1-2%:
In terms of establishing a benchmark suite... I think since
benchstat
allows as many units as desired, it'd be neat to flatten the data fromNIX_SHOW_STATS
to compare GC memory allocations. Likewise, including information about peak memory allocations, interrupts, etc. from GNU time.I'm not familiar with Meson by any means, and I don't know what would be involved in making such data available, but I assume there's a better way than transforming the build log printed to stderr. I had thought about making a separate output for the functional tests which stored the test results log file produced by Meson, but I don't know how that would interact with content-addressed stores (since rebuilds would have different paths because the times in the file would be different).
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.