NixOS always imports all modules

roberth commented 3 years ago

Describe the bug

NixOS always imports all modules, leading to degrading performance as NixOS grows.

Steps To Reproduce

Steps to reproduce the behavior:

Add many modules
Watch performance drop

But to really see the potential:

Make significant cuts in modules-list.nix and fix a few broken options references
Watch instantiation of toplevel go down from 4.0 to 1.4 seconds.

See proof of concept: https://github.com/hercules-ci/nixpkgs/commit/97dcbe8698b1332c90ba16cb5736e11d0097b061

Expected behavior

New modules don't reduce NixOS evaluation performance. NixOS can scale in complexity.

Additional context

I'll probably write an RFC suggesting the required refactorings and module authoring guidelines. This can all be achieved in a backcompatible manner, but the performance gains may be require some opting in.

Notify maintainers

@Infinisil

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
output here

Maintainer information:

# a list of nixpkgs attributes affected by the problem
attribute:
 - nixos
 - lib.nixosSystem
# a list of nixos modules affected by the problem
module: # most of them

jansol commented 3 years ago

I'm sure I already saw an issue or at least some discussion regarding this recently... Can't remember where exactly though.

roberth commented 3 years ago

It comes up in discussions every now and then, but usually it dies down because of scope creep and/or the prospect of a large and/or invasive change. I'd be interested to know of past issues, threads, etc, so I can address any concerns from those. I couldn't find an issue for this idea, but maybe I searched wrong.

jansol commented 3 years ago

Ah, it was this one: #57477

jbalme commented 3 years ago

The problem with modules is just loading them can have side effects (even though well-designed modules put all functionality behind enable flags.) This means they can't be lazily evaluated like packages, which would be the ideal imo.

rnhmjoj commented 3 years ago

This is also related to https://github.com/NixOS/nixpkgs/issues/79943 and RFC0022 as well.

roberth commented 3 years ago

This is gathering more attention than I expected.

@jbalme That's a correct intuition for why we can't fix this with a localized change in the module system in nixpkgs/lib.

@rnhmjoj Thanks!

It seems that these efforts were cut short because of flakes. Flakes are a fairly low level system of distributing expressions, not modules specifically. Flakes solve a problem that is largely independent: moving the files, whereas the performance can be fixed by refactoring the files. Such refactoring needs to happen before the modules can be moved, so it is not a wasted effort.

That said, I do think we tend to underestimate the value of having a somewhat coherent distribution of modules. We'll lose that if we switch entirely. Making broad changes to the NixOS modules will be much more painful because all the individual maintainers need to reinvent the such migrations. Flakes could be extended with versioning features perhaps to relieve some of the pain. Per-repo tooling is also a big cost.

What my issue is about

Basically, RFC 22 can be picked up. I've considered working on this issue in coming December, but the restructuring technique employed should be agreed on for such an effort to be effective. I'll describe the steps summarily here:

The first goal is to provide a new entrypoints into NixOS that do not make use of the complete module list. I'd probably start with a module that builds /etc and toplevel, which seem to be some of the least optional aspects of what NixOS can hypothetically be.

To bring the imports closure down to this minimum, I may encounter some dependencies on modules that are not essential to /etc and toplevel. These dependencies can be inverted, using new options if necessary. An alternative technique is to check options?foo.bar before defining anything (optionals, not mkIf!), but this is far less preferable. I believe this technique is only appropriate for out-of-tree compatibility logic.

With this core in place, new features can be refactored to make their closures minimal too.

Eventually, services can be added, with its infamously inefficient enable options. Instead of trying to model the problem in the module system somehow, I suggest moving away from this pattern. This can be done in a backwards compatible way. lib.nixosSystem will continue to function the way it does.

An illustration of what the imports relation looks like (arrows between boxes) and how they're referenced (arrows without boxes).

nixosConfigurations.my-laptop
          │
          │
          │
          └────────►lib.nixosSystem
                           │
                           │
                           │
                           │       ┌──────────────────┐      ┌───────────────────────────┐
                           └───────► modules-list.nix ├──────► services/foo/optional.nix │
                                   └──────────────────┘      └─┬───────────▲─────────────┘
                                                               │           │
                                             ┌─────────────────┘           │
 nixosConfigurations.some-server             │                             │
        │ │                        ┌─────────▼────────┐      ┌─────────────┴─────────────┐
        │ │                        │ systemd.nix      │      │ services/foo/default.nix  │
        │ │                        └─────────┬────────┘      └─────────────▲─────────────┘
        │ └────────►lib.nixosCore            │                             │
        │                  │                 │                             │
        │                  │                 │                             │
        │                  │       ┌─────────▼────────┐                    │
        │                  └───────► etc.nix          │                    │
        │                          └──────────────────┘                    │
        │                                                                  │
        │                                                                  │
        └──────────────────────────────────────────────────────────────────┘

Notable changes when applying this pattern

The nixos configuration of some-server needs to reference the foo service via imports, because it doesn't use modules-list.nix
The foo module was moved to foo/optional.nix. foo/default.nix imports foo/optional.nix and enables it. This is a low tech way to avoid having to enable services twice (imports, enable). A fancier solution is outside the scope, because it can be applied later with little effort.
Leaving systemd.nix out of nixosCore seems like wishful thinking today, but it lowers the bar for experimentation. The next init system tinkerer won't have to extract or reinvent /etc. Let's not focus on this though. It's more important that other system services can be included similarly.
Speaking of including other services. Sometimes we don't want to enable them, in which case we can either
- depend on an extracted interface (options in a separate module)
- only set the relevant config entries as follows optionals (options?services.someOptionalService) { services.someOptionalService.settings.foo.bar = true; } to avoid the otherwise problematic undeclared option errors

There's a couple of solutions and non-issues that I'm forgetting to write about right now.

What's next?

I'm glad I've shared my ideas, but I won't be able to spend much time at all on it in the coming months. On the plus side, this means that if you want to give it a go, it won't be a duplicated effort. Not that it would matter, because it's probably a good learning experience.

roberth commented 3 years ago

It is possible to do dynamic imports in a staged manner using submodules. However, I believe this to be a distraction because it is quite limiting and doesn't solve the problem completely, as we still have to process a list of all modules every time. Nonetheless, I'll include a proof of concept for completeness, and in case my assessment is wrong.

The significant limitation is that it is staged. The modules that are dynamically imported can not perform dynamic imports themselves. All the logic that could produce imports must reside at the root of the config tree, outside the configuration submodule in the proof of concept; a partitioned namespace apart from all the normal options. ~I am not convinced that this logic would amount to something different than building a closure of imports, just like imports already does by itself.~ This solution can provide a more predictable module ordering than using imports directly.

Proof of Concept: modules producing dynamic imports with a necessarily limited scope of writing

----- `staged-imports.nix` ```nix let coreModule = { lib, config, ... }: { options = { importList = lib.mkOption { type = lib.types.listOf lib.types.unspecified; }; configuration = lib.mkOption { type = lib.types.submoduleWith { modules = [ initSystemModule ]; }; }; }; config = { configuration = { ... }: { imports = config.importList; }; }; }; serviceModule = { lib, config, ... }: { options = { foo.enable = lib.mkEnableOption "foo"; }; config = lib.mkIf config.foo.enable { importList = [ fooModule ]; }; }; initSystemModule = { lib, ... }: { options = { services = lib.mkOption { type = lib.types.listOf lib.types.str; }; }; }; fooModule = { ... }: { config = { services = [ "foo" ]; }; }; # something like configuration.nix configurationModule = { ... }: { foo.enable = true; }; lib = import ./lib; in lib.evalModules { modules = [ coreModule serviceModule configurationModule ]; } ``` `terminal` ```console $ nix repl staged-imports.nix nix-repl> config.configuration.services [ "foo" ] ```

aanderse commented 9 months ago

hi @roberth :wave:

until this issue is resolved what is my best course of action? specify a baseModules argument to eval-config.nix which is a whittled down list of exactly what i need?

roberth commented 9 months ago

@aanderse While that is a possibility, it is a bit fragile, because it will not take into account new mandatory modules. Usually that leads to an evaluation error, but if not, it could be very bad for your deployment.

To do it properly, I think we should at least identify two sets of modules and separate those in Nixpkgs:

"core" modules that are always required, and which you will always want to import
"optional" modules that are imported by default when using the traditional entrypoints without "special" arguments such as baseModules, but which include everything from postgres to whatever obscure program or whatever.

That way, we put an upper bound on how bad it could get.

To make it work well in general, this concept needs more buy-in from the community.

I've done some experimental work which is only used in some tests as of now.

nixos.evalModules, which is meant as a minimal entrypoint, not unlike eval-config.nix, but actually minimal by default, and without inheriting legacy complexity. (nixos referring to inputs.nixpkgs.lib.nixos or import <nixpkgs/nixos/lib>)
nixosTests.etc and one or two other tests that only use this new "minimal" infrastructure.

While this works well enough, it is not well known. That's not a problem in itself, but something to be aware of. Also, as actually using this becomes feasible, we should document it. It's a bit of a chicken and egg problem to not document such experimental-ish things, so this lack of docs in the manual would be really good to start fixing.

The crucial thing that's missing right now, is an actually useful and meaningful imports graph. Currently, this graph is hardly a graph, but a very flat one that imports everything from basically the root. This is a local optimum where it is simple, but importing anything but that root is not guaranteed to be complete. In order to use make effective use of optional modules, they need to have a reasonably complete imports, and this should be tested. So one of the ways forward is to start writing NixOS tests in this way. (ie extend nixos/lib/testing/nodes.nix to allow use of a different entrypoint and start using that in tests. For this in turn it'd help a lot to make test matrixes easier, because we need both traditional and minimal usages to work - although that's falling to deep into the rabbit hole perhaps? Nonetheless, see https://github.com/NixOS/nixpkgs/pull/176557#issuecomment-1554080211) To make the testing more effective, it would also help a lot if we could make ofborg aware of a mapping from files to relevant tests. Package-based change detection is not going to trigger the right tests for NixOS, if any at all.

By solving these problems, we have a coherent story for supporting both traditional and minimal use of modules, and we can explain why the small changes we need are useful. For instance, adding traditionally redundant imports currently leads to some resistance and some might even "clean them up".

aanderse commented 9 months ago

thanks for the helpful response @roberth - as always, very thorough :heart:

Aleksanaa commented 5 months ago

This logic is not only related to performance. It can be problematic (and even dangerous sometimes)

See an example just found: https://github.com/NixOS/nixpkgs/issues/305304

Atemu commented 3 months ago

Would it be possible to construct the import graph using the module system's existing options?

Whenever a service needs i.e. postgres, they set services.postgres.enable = true. Couldn't we make it so that, initially, the posgres module only exists as a stub .enable option which, upon enablement, imports the actual postgres module?
That would side-step the issue of individual modules needing to explicitly declare dependencies for non-core modules because they already do so via enable options.

Of course such optional modules would have some limitations such that config they set is only active when the enable option is true.
Most service modules we have are already set up like that however (or could trivially be converted) as this is an existing convention in NixOS. All of these would just be made to behave lazily like that at essentially no cost.

That wouldn't be a silver bullet fixing performance issues all at once but it should at least remove an entire category of modules (optional services you don't care about) from the user's module system eval.
It'd also open a path towards incremental progress by people converting more modules to such "standard" modules (guarded behind an import condition) to gradually improve performance.

A further step that could be done would be to broaden the enablement condition to also include arbitrary options under the module's "namespace" being set. The foo module could be imported when services.foo != { } for instance. This would be handy for modules that don't have a typical .enable option but are only active when some option is in a non-default config.
This would again rely on existing conventions (everything under an attr is handled by one module) but I think that's at the very least a good starting point.

eclairevoyant commented 3 months ago

Couldn't we make it so that, initially, the posgres module only exists as a stub .enable option which, upon enablement, imports the actual postgres module?

I have to note, conditional importing sounds like an absolute nightmare for debuggability

Atemu commented 3 months ago

What specific issues do you forsee?

It should also be possible to have an option to turn this optimisation off and always import the full list of modules, regardless of enablement.

I also found out that I wasn't the first person to think of this general idea: https://github.com/NixOS/nixpkgs/issues/57477#issuecomment-816881802

Aleksanaa commented 3 months ago

Whenever a service needs i.e. postgres, they set services.postgres.enable = true. Couldn't we make it so that, initially, the posgres module only exists as a stub .enable option which, upon enablement, imports the actual postgres module?

I totally thought so. Let's talk about it in a few days.

roberth commented 3 months ago

@Atemu it requires that you either have

Fixpoint iteration on top of the fixpoint we have

Perform fixpoint iteration on top of the lazy fixpoint we already have for the config. This may indeed be horrible for debugging, because you would have to consider whether the error occurs during which phase or iteration. Also fixpoint iteration relies on a termination condition that produces a boolean, so you'd have to make huge == comparisons for each iteration, and you can't compare the whole config, because the config is designed to be lazy. Some types aren't even comparable, such as functionTo and deferredModule. You'd have to figure out which options matter and treat them specially, at which point you're implementing an unnecessarily complicated version of the alternative, which is...

Better use of submodules

Do the expensive stuff in a submodule. It is perfectly fine for a module to control what goes into its submodule(s), with all sorts of conditions and whatnot, as long as those don't depend on the evaluation of that submodule. This way we don't need new complicated generic machinery; the module system tools we have suffice.

This is also what I've proposed here. The "portability" aspect of that may well be a minor detail. I've jokingly referred to it as the Importable Service Layer, and if you un-split some modules and rename some things, that's actually what it is.

It does cause changes to the option structure, but that is actually beneficial because it adds the ability to have multiple instances of a service to all services in a consistent manner. It's also possible to remain interface compatible by creating a few options that forward their definitions to a service module with a hardcoded "default" name, such as "cups". This has the effect of:

Whenever a service needs i.e. postgres, they set services.postgres.enable = true. Couldn't we make it so that, initially, the posgres module only exists as a stub .enable option which, upon enablement, imports the actual postgres module?

eadwu commented 1 month ago

I think it is a good idea to define a base set of modules.

For some reason the set of necessary modules for evaluation != the set of modules needed for a proper system. Or rather, the minimal set of modules for evaluation.

eclairevoyant commented 1 month ago

For some reason the set of necessary modules for evaluation != the set of modules needed for a proper system.

For eval this is currently all you need, more or less: https://github.com/eclairevoyant/nix-templates/blob/main/nixos/flake.nix#L12-L22

What you mean by the "set of modules needed for a proper system" is unclear here. (I'm also unclear on how that related to this issue.)

eadwu commented 1 month ago

If you try to minimize the number of modules imported as the metric for reducing modules, you will end up with an inoperable system, i.e. in the event where there is no option evaluated and the config is always active { config = { environment.systemPackages = [ pkgs.ssss ]; }; } as you can remove any of those modules safely without breaking evaluation.

Your profile does not reduce the number of modules imported.

NixOS / nixpkgs