-
-
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
Cache the result of getFlake #4511
base: master
Are you sure you want to change the base?
Conversation
28c7ecc
to
056d419
Compare
I've update this to cache a few more things (in particular the stringification of derivations, which is probably the most important thing to cache). I've also run some quick benchmarks, and the results are not really nice as the caching adds a noticeable overhead to the evaluation (but it's very likely that it can be optimized. I'll look into that in the coming days). The results are below. The command I ran for the benchmarks is
where let
pkgs = (builtins.getFlake "github:NixOS/nixpkgs?rev=ad0d20345219790533ebe06571f82ed6b034db31").legacyPackages.x86_64-linux;
in
pkgs.runCommandNoCC "foo" {
buildInputs = [
pkgs.firefox
pkgs.pandoc
];
}
"echo bar > $out"
|
056d419
to
45dda96
Compare
Hook into the evaluation to cache to cache the evaluation of flake members (stuff like `(builtins.getCache foo).bar.baz`). Also replaces the old eval cache that was used at the cli level.
d7bc97c
to
357e095
Compare
After a lot of struggle and rewriting the cache partially from scratch, I eventually managed to get some much nicer benchmark results: On the evaluation part the overhead is down to between 0 and 10% (generally around 5%) when the cache is disabled or cold. On the exact same benchmark, I get:
(with the caveat that the relative speed of the |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/tweag-nix-dev-update-7/11552/1 |
Do you see a way how we could partition nixpkgs evaluation in nixpkgs-review using the cache? |
Forcibly coerce the `path` argument to a string to make it work even if it's a path
85ea10c
to
531cc65
Compare
Because of a dirty limit of the `std::visit(overloaded` trick, these were silently casted to booleans
Good question… This PR alone wouldn't do it, but I guess this + a |
`nix flake show --legacy` was aborting the evaluation a bit too eagerly in case of error, so that running it on nixpkgs was only showing empty packages sets. Catch the error earlier to allow displaying the full set of packages
Ok. All we would need for nixpkgs-review is the store path and if possible filter output to only include architecture. However from what I currently see |
I marked this as stale due to inactivity. → More info |
Add an evaluation caching mechanism
Add the caching mechanism already present for cli arguments to the evaluation loop.
This means that an expression of the form
(builtins.getFlake foo).bar.baz
will be cached if it can be (and also slightly more complex stuff likelet pkgs = (builtins.getFlake foo).packages.x86_64; in pkgs.bar
.This isn't as useful as it could be because flake inputs aren't cached (yet). But it opens the way for it.