From afecbb2f7534a0530fc0da53af74d0bb85697d36 Mon Sep 17 00:00:00 2001 From: pennae Date: Fri, 24 Dec 2021 22:36:08 +0100 Subject: [PATCH 1/3] lib/modules: optimize byName the foldl is equivalent to a zip with concat. list concatenation in nix is an O(n) operation, which makes this operation extremely inefficient when large numbers of modules are involved. this change reduces the number of list elements by 7 million on the system used to write this, total memory spent on lists by 58MB, and total memory allocated on the GC heap by almost 100MB (with a similar reduction in GC heap size). it's also slightly faster. --- lib/modules.nix | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 573bf40e4b34..091c72e30aae 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -37,6 +37,7 @@ let toList types warnIf + zipAttrsWith ; inherit (lib.options) isOption @@ -442,7 +443,8 @@ rec { } */ byName = attr: f: modules: - foldl' (acc: module: + zipAttrsWith (n: concatLists) + (map (module: if !(builtins.isAttrs module.${attr}) then throw '' You're trying to declare a value of type `${builtins.typeOf module.${attr}}' @@ -454,11 +456,8 @@ rec { this option by e.g. referring to `man 5 configuration.nix'! '' else - acc // (mapAttrs (n: v: - (acc.${n} or []) ++ f module v - ) module.${attr} - ) - ) {} modules; + mapAttrs (n: f module) module.${attr} + ) modules); # an attrset 'name' => list of submodules that declare ‘name’. declsByName = byName "options" (module: option: [{ inherit (module) _file; options = option; }] From 2dcae7d82f5ef0b373413d2fbfc1001141561c74 Mon Sep 17 00:00:00 2001 From: pennae Date: Sat, 25 Dec 2021 15:20:26 +0100 Subject: [PATCH 2/3] lib/attrsets: use builtins.zipAttrsWith if available --- lib/attrsets.nix | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/attrsets.nix b/lib/attrsets.nix index 812521ce6d1c..4721c833337e 100644 --- a/lib/attrsets.nix +++ b/lib/attrsets.nix @@ -378,7 +378,8 @@ rec { zipAttrsWith (name: values: values) [{a = "x";} {a = "y"; b = "z";}] => { a = ["x" "y"]; b = ["z"] } */ - zipAttrsWith = f: sets: zipAttrsWithNames (concatMap attrNames sets) f sets; + zipAttrsWith = + builtins.zipAttrsWith or (f: sets: zipAttrsWithNames (concatMap attrNames sets) f sets); /* Like `zipAttrsWith' with `(name: values: values)' as the function. Example: From 0de4ecff8cbeb9c3f36e58b3bdb406d676a5e118 Mon Sep 17 00:00:00 2001 From: pennae Date: Tue, 28 Dec 2021 16:53:50 +0100 Subject: [PATCH 3/3] lib/modules: extract multiply-used value in byName module.${attr} is used at least twice, so it must be evaluated at least twice (and since it's a function argument, be turned into a thunk twice). --- lib/modules.nix | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/modules.nix b/lib/modules.nix index 091c72e30aae..c68bbfcaa3e0 100644 --- a/lib/modules.nix +++ b/lib/modules.nix @@ -444,10 +444,10 @@ rec { */ byName = attr: f: modules: zipAttrsWith (n: concatLists) - (map (module: - if !(builtins.isAttrs module.${attr}) then + (map (module: let subtree = module.${attr}; in + if !(builtins.isAttrs subtree) then throw '' - You're trying to declare a value of type `${builtins.typeOf module.${attr}}' + You're trying to declare a value of type `${builtins.typeOf subtree}' rather than an attribute-set for the option `${builtins.concatStringsSep "." prefix}'! @@ -456,7 +456,7 @@ rec { this option by e.g. referring to `man 5 configuration.nix'! '' else - mapAttrs (n: f module) module.${attr} + mapAttrs (n: f module) subtree ) modules); # an attrset 'name' => list of submodules that declare ‘name’. declsByName = byName "options" (module: option: