local evaluation: use nixpkgs ci.eval parallel implementation#440
local evaluation: use nixpkgs ci.eval parallel implementation#440GaetanLepage wants to merge 2 commits intoMic92:mainfrom
Conversation
f1062d7 to
87d78c6
Compare
3a148ac to
42dd4c6
Compare
|
For now, I initialize the parameters following the recommendations of the
I conducted early and naive benchmarks on the following system:
I ran the evaluation for the four supported systems (
So unfortunately, it appears that this method is slower than our current one... Maybe tuning the hyper-parameters can give better results, but as we are maxing out the CPU, I don't expect a lot of gains. Note: using cc @infinisil, maybe you will have more insights. |
|
You should be able to get the same speed as the legacy |
|
How evenly is the max memory usage spread across different chunks? |
I don't really know. The overall RAM usage fluctuates and peaks very briefly at ~39GB (using |
|
It feels like we need to solve this in nix. i.e. track RAM usage there and restart the interpreter after a threshold is breached. |
|
So, what should we do about this ? |
Maybe the new method could be a different branch for the time being. Some people might have machines with enough resources, where it actually speeds up things. I am wondering if we can leverage |
Did you try my suggestion of having a single huge chunk, so that it should effectively be the same single-threaded performance as before? |
42dd4c6 to
fef5490
Compare
fef5490 to
308620a
Compare
Sorry, I forgot to follow up. Here is the benchmark I made:
Well...it OOMed :'( And it was not done with the first (pre-change) evaluation... |
|
I can only imagine that nixpkgs-review doesn't do the same eval then |
This PR aims at improving local evaluation.
Currently, it uses a (single-threaded)
nix-envcall per system to evaluate.This new patch leverages the recent
ci.evalparallel evaluation implementation innixpkgsitself.Current limitations:
--allowas we used to (maybe it is possible...)--max-jobs,--coresandchunkSizeparameters ofci.fullWe need to expose new options for those and provide sensible (automatic ?) default values.
stdoutis now limited to listing all "rebuilds" (i.e. added + changed packages). Before, it was more detailed (removed, changed and added I think). This information is not available in the output of theeval.comparederivation. If needed, we could change the upstream implem...TODO
--allowuser preferenceseval.fullparameters (--max-jobs,--coresand--arg chunkSize)cc @Mic92 @zowoq @khaneliman