| 4 May 2023 |
 hexa | ok, just making sure | 21:38:58 |
 raitobezarius | I'm trying to understand the options | 21:39:07 |
| raitobezarius | Clearly, if I had a way to relate changed paths with those paths in the JSON, it would be easy win | 21:39:25 |
| raitobezarius | But I think it's a good idea to try to be proper about it | 21:40:17 |
| raitobezarius | now that I talked enough I have an idea to do it purely in Nix maybe | 21:46:46 |
| raitobezarius | ah but your technique cannot recover the name of the module | 21:54:32 |
| raitobezarius | also I don't think we store the name of modules anywhere anyway | 21:54:38 |
| raitobezarius | https://github.com/NixOS/ofborg/pull/641 here's a wip | 22:09:05 |
| raitobezarius | some stuff might need to be fixed but I think the strategy should work | 22:09:30 |
| raitobezarius | interested into some feedback though before going too far | 22:10:21 |
| raitobezarius | In reply to @raitobezarius:matrix.org
also I don't think we store the name of modules anywhere anyway (I can derive it via string ops on filenames) | 22:11:39 |
 cole-h | Having to evaluate NixOS seems like it would add a lot of overheard to the evaluation times. Maybe you could look at how it would change that before going too far.
ofborg/src/nixstats.rs and ofborg/src/outpathdiff.rs might have some hints on how that information is generated. | 22:12:11 |
| raitobezarius | evaluating NixOS with an empty module set I believe is extremely fast? | 22:12:44 |
| raitobezarius | * evaluating NixOS with an empty configuration I believe is extremely fast? | 22:12:51 |
| cole-h | I'd be happier with empirical evidence :P | 22:13:05 |
| raitobezarius | ❯ hyperfine "nix-instantiate --eval '<nixpkgs/nixos>' --arg configuration '{}' -A config.meta.maintainers --strict --json"
Benchmark 1: nix-instantiate --eval '<nixpkgs/nixos>' --arg configuration '{}' -A config.meta.maintainers --strict --json
Time (mean ± σ): 621.8 ms ± 22.6 ms [User: 524.8 ms, System: 91.2 ms]
Range (min … max): 597.3 ms … 663.9 ms 10 runs
| 22:13:59 |
| raitobezarius | {
"cpuTime": 0.49957001209259033,
"envs": {
"bytes": 12506824,
"elements": 609381,
"number": 476986
},
"gc": {
"heapSize": 402915328,
"totalBytes": 87726480
},
"list": {
"bytes": 1933232,
"concats": 13049,
"elements": 241654
},
"nrAvoided": 488806,
"nrFunctionCalls": 412021,
"nrLookups": 240540,
"nrOpUpdateValuesCopied": 1193821,
"nrOpUpdates": 12482,
"nrPrimOpCalls": 244606,
"nrThunks": 691079,
"sets": {
"bytes": 29699424,
"elements": 1702651,
"number": 153563
},
"sizes": {
"Attr": 16,
"Bindings": 16,
"Env": 16,
"Value": 24
},
"symbols": {
"bytes": 531386,
"number": 45882
},
"values": {
"bytes": 27441768,
"number": 1143407
}
}
| 22:14:20 |
| raitobezarius | for the nix show stats | 22:14:23 |
| cole-h | Cool | 22:14:35 |
| raitobezarius | I mean I'm not aware of the performance constraints of ofborg, but I believe it's probably negligible? | 22:15:03 |
| cole-h | Not necessarily constraints, but when you start evaluating every PR on all of Nixpkgs, slowdowns can be noticeable | 22:16:00 |
| raitobezarius | Of course | 22:16:10 |
| cole-h | I'll take a closer look tomorrow, but it doesn't look immediately flawed, as long as performance doesn't massively regress :P | 22:18:35 |
| raitobezarius | :) | 22:18:52 |
| raitobezarius | let me know | 22:18:55 |
| raitobezarius | I wished we could just run performance testing without setting the whole broker/worker stuff :D | 22:19:13 |
| 5 May 2023 |
| raitobezarius | https://github.com/NixOS/ofborg/pull/562/files interesting PR to revive | 20:40:39 |
| 6 May 2023 |
 Lily Foster | Ofborg's ofborg-eval-package-list-no-aliases check is still missing a lot of alias usage (see https://github.com/NixOS/nixpkgs/pull/230188#pullrequestreview-1415772214)
I notice that adding --drv-path to the command here https://github.com/NixOS/ofborg/blob/0f34038feb9b0ae9959c865608700c91d57b2590/ofborg/src/tasks/eval/nixpkgs.rs#L455 lets it catch those, but obviously that makes it a more expensive eval (since it has to actually evaluate the entire drvs)
Is there a better way we can detect alias usage instead of having it pass PRs that fail with allowAliases = false?
| 14:13:15 |
| Lily Foster | (I am also happy to open a PR to add --drv-path to that check, and just discuss it there. I just don't know much ofborg and idk how reasonable that solution is) | 14:17:17 |
 Vladimír Čunát | The most expensive checks that I'm aware of running anywhere need around 36G RAM: https://github.com/NixOS/nixpkgs/issues/227945 | 14:53:11 |
