-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
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
nix-daemon: Add CPU and IO weight options #26520
Conversation
@Baughn, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @MarcWeber and @abbradar to be potential reviewers. |
Fwiw the nix daemon should be prioritized like a user facing application, because indeed for operations like |
20% by default, maybe? I don't know about OSX, but using CPUWeight it'll still use any idle capacity. I'm not seeing any effect on shell usage, but... hmm, this is hard. Ideally it'd depend on which window is in focus. |
@@ -725,6 +725,8 @@ in | |||
|
|||
"systemd/system.conf".text = '' | |||
[Manager] | |||
DefaultCPUAccounting=yes | |||
DefaultBlockIOAccounting=yes |
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.
BlockIOAccounting
refers to the legacy cgroup hierarchy (while IOWeight
used above belongs to the unified hierarchy).
The way I read the manual, this is strictly redundant: enabling a type of resource control for one unit is said to imply enabling it for the slice it runs in and all parent slices.
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.
When testing on my own system, the CPU accounting didn't seem to work without this setting. (But worked just fine even though I accidentally put it in user.conf instead of system.conf the first time, so I'm not sure what's going on.)
I can't test the IO accounting, you may well be right about that. What should I be using instead? My manual (man systemd-system.conf) just lists this one.
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.
All the BlockIO*
directives are now just IO*
, so you can use DefaultIOAccounting
.
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.
The manual can explain it better: https://www.freedesktop.org/software/systemd/man/systemd.resource-control.html#CPUAccounting=
I feel that we should not provide per-service options that duplicate systemd resource management functionality, because then we could potentially be adding hundreds of such options. Instead users should just use the systemd options, e.g.
( I have no strong feelings about turning on accounting by default, but if it's not the upstream default, then maybe we shouldn't. BTW, according to the systemd docs, CPUWeight and IOWeight require the unified control group hierarchy. But, AFAIK, we don't enable that yet. (But maybe it works in the hybrid mode, which is the systemd 233 default, I think.) |
If |
@edolstra My reasoning for adding the option is that without it, the default experience for NixOS is that the rest of the system gets laggy while nix-build is running. Which can happen as part of auto-updates. I have no objection to using common options, but I feel that any such solution ought to have the same defaults as this patch. |
@edolstra So let's talk interfaces. The reason I'd like to add the options is so people will know they exist, and because there already is a niceness option there. With reasonable defaults that becomes less important, but my first thought on seeing "Huh, nix-build is eating all my CPU time" was to look for options relating to nix-build, not systemd. On the other hand, long-term it would be good if systemd is the first place people look. It just isn't the case yet, since we haven't had these tools for very long. How would you feel if, as a compromise:
|
If you are just a user of the system and don't know anything about NixOS, you want it to run the daemon in such a way that you never notice it. If you are a user who wants to use nix-shell, because you want to develop something and you are waiting for the answer, you want it to be done as fast as possible. Any design which doesn't meet these two use cases is a design that isn't worth implementing. |
(triage) @edolstra, do you have an answer for #26520 (comment) ? |
FWIW, since I upgraded to a TR4 1950X, I am no longer able to test this patch. For other reasons[1] I cannot configure Nix such that it ever uses the full CPU capacity of my machine. [1] Specifically, the build daemon uses a ton of memory. So do compilers, etc. 32GB of memory isn't enough to gain 32-way parallelism in the builds; I have to configure it with a max of 12 threads. |
If people have a hard time knowing that they can just use |
Closing this due to reasons mentioned above, see #26520 (comment) for how to do it without this option. |
Motivation for this change
Making Chrome (or other UI elements) less laggy.
CPU prioritization by way of 'nice' doesn't work very well. Modern systems have a better option, in that systemd can be used to set CPU timeslice weighting explicitly. This requires turning on accounting for the same, globally across the system, which I do with the systemd config patch.
The second patch makes the CPU weighting configurable for the nix-daemon service, and defaults it to the minimum possible. I believe this is a good default for desktop systems, as well as most servers; the builds are generally batch processes, and this setting does not prevent it from using all idle time.
All of the above is duplicated for I/O accounting, although I cannot test the impact from this due to running on NVMe.
There is a very slight performance impact from the accounting, but not so you'd notice. My experience is that it (if it exists at all) is vastly outweighed by usability improvements through making interactive sessions higher priority.
(I've been told that CPU and I/O accounting has low impact, while memory accounting has medium. This patch does not enable memory accounting.)
Other batch services should probably get the same treatment.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)