-
-
Notifications
You must be signed in to change notification settings - Fork 14.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
Ensure all jobset attributes evaluate, and check that in CT #269356
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
6.topic: nixos
Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS
6.topic: GNOME
GNOME desktop environment and its underlying platform
6.topic: haskell
6.topic: qt/kde
6.topic: emacs
Text editor
6.topic: rust
6.topic: erlang
6.topic: steam
Steam game store/launcher (store.steampowered.com)
6.topic: lua
6.topic: module system
About "NixOS" module system internals
6.topic: lib
The Nixpkgs function library
labels
Nov 23, 2023
ghost
linked an issue
Nov 23, 2023
that may be
closed
by this pull request
ghost
changed the title
release-attrpaths-superset.nix: 46x faster, 1/30th the peak memory
release-attrpaths-superset.nix: 33x faster, 1/30th the peak memory
Nov 23, 2023
ofborg
bot
added
10.rebuild-darwin: 0
This PR does not cause any packages to rebuild on Darwin
10.rebuild-linux: 1-10
labels
Nov 23, 2023
ghost
mentioned this pull request
Nov 23, 2023
MeasurementsAccording to GNU Time
Compared to
|
ghost
marked this pull request as ready for review
November 23, 2023 13:27
ghost
requested review from
piegamesde,
jtojnar,
adisbladis,
ttuegel,
edwtjo,
Mic92,
zowoq,
winterqt,
figsoda,
stigtsp,
zakame and
dasJ
as code owners
November 23, 2023 13:27
…ckages, most of them are broken Since Hydra does not build unfree packages an astonishing proportion of them are broken yet not marked meta.broken.
Rebased. |
This file walks the entire nixpkgs tree and emits a superset of all release attrnames in only 44 seconds on a 3ghz CPU, using 5 gbytes of memory. By comparison, on the same CPU the `nix-env` hack used by ofborg on every PR submission requires 41 *minutes* and peaks at 60 gbytes, even with checkMeta turned off. Full details below. This is: - 46x faster (or 2.1% of the elapsed time) - 12.5x less memory (or 8.0% of the peak memory usage) In order to replace the ofborg check, this list of attrnames must then be post-filtered for platform-relevance. However, crucially, the post-filtering can be done *in parallel* on multiple cores by splitting the attrname list in to chunks. Generating the list of attrnames cannot be parallelized because it is a single-threaded cppnix task. This PR also adds `recurseForDerivations` where necessary within nixpkgs in order to make this possible -- it screens out various non-tryEval-catchable failures and infinite recursions. Before undraftifying, I will add an invocation of this command to the CI tests, to ensure that the work performed here is not immediately undone. My next PR will then add an additional CI check confirming that the emitted attrpaths are in fact a superset of the release attrpaths calculated by the slow-memory-hog ofborg method. I have manually confirmed that this is the case at the tip commit of this PR, but we need CI to make sure this remains true until ofborg switches to this more-efficient method of calculation; at that point the superset-check can be dropped. According to GNU Time, Command being timed: "nix-instantiate --eval --strict --json pkgs/top-level/release-attrpaths-superset.nix -A names" User time (seconds): 44.88 System time (seconds): 8.09 Percent of CPU this job got: 99% Elapsed (wall clock) time (h:mm:ss or m:ss): 0:53.20 Average shared text size (kbytes): 0 Average unshared data size (kbytes): 0 Average stack size (kbytes): 0 Average total size (kbytes): 0 Maximum resident set size (kbytes): 4823028 Average resident set size (kbytes): 0 Major (requiring I/O) page faults: 0 Minor (reclaiming a frame) page faults: 3611240 Voluntary context switches: 113 Involuntary context switches: 949 Swaps: 0 File system inputs: 1480 File system outputs: 5944 Socket messages sent: 0 Socket messages received: 0 Signals delivered: 0 Page size (bytes): 4096 Exit status: 0 Compared to release-outpaths.nix: Command being timed: "nix-env -qaP --no-name --out-path --arg checkMeta false --argstr path /git/work/pr/release-outpaths -f pkgs/top-level/release-outpaths.nix" User time (seconds): 2120.67 System time (seconds): 337.80 Percent of CPU this job got: 98% Elapsed (wall clock) time (h:mm:ss or m:ss): 41:37.91 Average shared text size (kbytes): 0 Average unshared data size (kbytes): 0 Average stack size (kbytes): 0 Average total size (kbytes): 0 Maximum resident set size (kbytes): 60171768 Average resident set size (kbytes): 0 Major (requiring I/O) page faults: 2 Minor (reclaiming a frame) page faults: 230608113 Voluntary context switches: 8876 Involuntary context switches: 22275 Swaps: 0 File system inputs: 62624 File system outputs: 72 Socket messages sent: 0 Socket messages received: 0 Signals delivered: 0 Page size (bytes): 4096 Exit status: 0
This derivation verifies that all jobset attributes can be evaluated under tryEval without producing any non-catchable errors or causing infinite recursion.
This commit temporarily adds pkgs/test/release to the lib/tests/release.nix test suite, because ofborg already knows about that entry point. We should move the list of test entry points out of ofborg and into a central place in nixpkgs: #272591 Once we do that we won't need to have this ugly kludge in an inappropriate place.
…unsupported" We have packages that use `meta.platforms = []` as a sort of synonym for `broken = true`. Without this commit, the attrnames for those jobs will end up in the list of attrnames which are expected to build, even though they are not expected to build.
infinisil
approved these changes
Dec 15, 2023
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.
Looking good to me, tested locally. To prevent further conflicts I'll merge now! 🚀
Edit: Hold on, I'll also quickly test this on Darwin
ghost
deleted the
pr/release-outpaths-
branch
January 23, 2024 06:49
This was referenced Jul 1, 2024
Merged
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
6.topic: emacs
Text editor
6.topic: erlang
6.topic: GNOME
GNOME desktop environment and its underlying platform
6.topic: haskell
6.topic: lib
The Nixpkgs function library
6.topic: lua
6.topic: module system
About "NixOS" module system internals
6.topic: qt/kde
6.topic: rust
10.rebuild-darwin: 0
This PR does not cause any packages to rebuild on Darwin
10.rebuild-linux: 1-10
significant
Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
aka "eval checks go brrrr"
Please review the commits individually. Produced as part of the non-Hydra tooling used to validate
Description of changes
This PR adds
release-attrpaths-superset.nix
, which walks the entire nixpkgs tree and emits a superset of all release attrnames in only 44 seconds on a 3ghz CPU, using less than 5 gbytes of memory. By comparison, on the same CPU thenix-env -qaP
hack used by ofborg on every PR submission requires waiting 25 minutes and peaks at 58 gbytes, even with checkMeta turned off. Full details below. This is:If all you need is the attrnames -- for example to launch a mass rebuild -- then you're done. If you want to replace the ofborg
outpaths.nix
step, see the follow-up PR:Validation
To confirm that the
release-attrpaths-superset.nix
really are a superset of the slower method, runAdditional CT (Continuous Testing) check
This PR adds
recurseForRelease=false
where necessary within nixpkgs in order to make this possible -- it screens out infinite recursions and several non-tryEval
-catchable failures (for example, passing a mistyped argument to a builtin, which is an uncatchable error) and infinite recursions.Finding all the places where it was necessary to add this was a lot of tedious work, so to make sure that work is not undone,
lib/tests/release.nix
now verifies thatrelease-attrpaths-superset.nix
evaluates successfully.Although it doesn't really fit in
lib/tests
, there isn't anywhere else it can be placed and still be run by ofborg on every commit. If some other such place materializes in the future I would be happy to relocate it, but since ofborg is out-of-tree this PR should not block on that happening.Fixes