nixos/qemu-vm: Allow building a Nix store image instead of relying on 9p + more by talyz · Pull Request #140792 · NixOS/nixpkgs

talyz · 2021-10-06T18:51:01Z

Motivation for this change

The goal of this PR is to speed up some IO heavy tests by building a Nix store disk image instead of using 9p to access the host store.

To this end, the following things are done:

In make-disk-image.nix:
- Add the onlyNixStore argument, which allows building an image containing only the Nix store
- Reintroduce the installBootLoader argument, which seemed to have been accidentally removed
- Add the copyChannel argument, to control whether a default nix channel is copied to the image
- Add the additionalPaths argument to allow additional paths to be copied to the image's nix store
In qemu-vm.nix:
- Add the useNixStoreImage option, which controls whether an image is used rather than 9p
- Rename the pathsInNixDB option to the more general additionalPaths, since its purpose is different when building an image
In trivial-builders.nix:
- Add writeStringReferencesToFile, a builder which extracts a string's references to derivations and paths and writes them to a text file, removing the input string itself from the dependency graph
In testing-python.nix:
- Make sure all derivations referenced by the test script are added to additionalPaths, since they need to be explicitly made available when building an image

These changes are then put to use in the discourse and gitlab tests. With my laptop (AMD Ryzen 7 PRO 2700U) as the reference system, I see the following test run times:

Discourse
- No change:
  25 mins, 49 secs
- Building a Nix store image:
  4 mins, 44 secs
- Building a Nix store image and bumping the core count to 4:
  3 mins, 6 secs
GitLab
- No change:
  Times out after 28 mins
- Building a Nix store image:
  7 mins, 48 secs
- Building a Nix store image and bumping the core count to 4:
  7 mins, 17 secs

The times include the time it takes to build the image (~40 secs for discourse and ~1 min, 20 secs for gitlab).

Things done

nixos/lib/make-disk-image.nix

nixos/modules/virtualisation/qemu-vm.nix

roberth · 2021-10-07T20:13:36Z

I wonder if something like fscache could be a solution. Potentially it gives the best of both worlds: not building an entire image, yet avoiding 9p for repeated accesses. Could you compare performance with cache=fscache instead of cache=loose?

talyz · 2021-10-07T21:50:24Z

@roberth Tried setting cache=fscache now: for discourse, it gave me a run time of 25 mins, 45 secs, so probably within error.

mohe2015 · 2021-10-07T22:04:24Z

I have no idea but could https://virtio-fs.gitlab.io/ be an alternative?

talyz · 2021-10-07T22:41:12Z

@mohe2015 Yeah, I was considering it, but figured it wouldn't give quite the same performance boost, plus it needs a userspace daemon running on the host, so this seemed like the safer bet. It could certainly be interesting to try, though - if the benchmarks are correct, it should give a decent boost over 9p :)

knedlsepp · 2021-10-08T12:09:04Z

I'm curious about the downsides of this. Will this increase disk usage or network traffic on hydra or the binary-cache? Do we need to worry about that?

andir · 2021-10-08T12:11:57Z

I have no idea but could https://virtio-fs.gitlab.io/ be an alternative?

That currently requires cooperation from the Host OS as virtiofs requires a daemon running as root (and a socket available to QEmu during execution).

mohe2015 · 2021-10-08T13:18:56Z

Yeah, that was already mentioned above. As far as I can tell this solution has the disadvantage of more disk usage but the performance is just so much better

talyz · 2021-10-08T14:02:52Z

@knedlsepp AFAIK, this should only increase disk usage on the builder. The disk image doesn't end up in the test's closure and I don't see why any of it should end up in the binary cache. Maybe someone with more insight into hydra can chime in, though :)

talyz · 2021-10-15T08:26:20Z

Anything more to do here? Any objections to merging it?

roberth

Found a potential simplification. Otherwise lgtm!

nixos/lib/testing-python.nix

talyz · 2021-10-28T10:28:53Z

Okay, now it's done!

roberth · 2021-10-28T10:32:23Z

nixos/modules/virtualisation/qemu-vm.nix

-      shared    = { source = ''"''${SHARED_DIR:-$TMPDIR/xchg}"''; target = "/tmp/shared"; };
+      nix-store = mkIf (!cfg.useNixStoreImage) {
+        source = builtins.storeDir;
+        target = "/nix/store";


Suggested change

target = "/nix/store";

target = "/nix/store"; # should be storeDir, but is not supported in scripts yet

Well, technically, it should be set to whatever storeDir was set to in the nix derivation used in the vm, which currently isn't exposed, but could be added to its passthru attributes.

If it was possible, that would be the solution, but you can't choose storeDir from inside the Nix language.

Oh yeah, you're right. I guess the reasonable thing to do then is just un-hardcode all the values.

…n useNixStoreImage = true The involved test was nixosTests.nextcloud.basic21. It has a testScript that is strict in nodes.nextcloud.config.system.build.vm, in assertions about imagemagick being in the system closure. The recursion was introduced in 329a446 from NixOS#140792

roberth · 2021-10-31T18:09:19Z

While #144014 breaks the infinite recursion when useNixStoreImage is disabled, the same could occur in a test that does use it. We can use two basic techniques:

shadow known bad config values using // and whatnot. This allows the expression to fail before it would enter into infinite recursion.
don't pass the real nodes into testScript, but a very similar set of nodes with additionalPaths mkForced to [].

The first has no performance impact. The second does.
If we combine them, we only have a performance impact when necessary, but we lose a bit of safety when it comes to new options in the future.

talyz · 2021-11-01T11:17:29Z

@roberth It seems like hydra doesn't want to build the disk images due to their size. Raising the max_output_size is probably the solution. Do you know if this is doable? Also, do you know if the disk image would end up in the cache even though it's not part of the final closure?

roberth · 2021-11-01T17:08:52Z

Raising the max_output_size

This may be a hard sell.

if the disk image would end up in the cache even though it's not part of the final closure?

Not 100% sure, but I believe it caches everything it builds. You generally don't want to throw away your tools.

talyz · 2021-11-01T18:01:03Z

Raising the max_output_size

This may be a hard sell.

A shame if it's true, since that would mean all tests using this will always fail :/

if the disk image would end up in the cache even though it's not part of the final closure?

Not 100% sure, but I believe it caches everything it builds. You generally don't want to throw away your tools.

True, but I would have guessed that all tools we want to keep are primary inputs anyway. It would be interesting to know if there are any attributes that control this, like preferLocalBuild or something.

talyz · 2021-11-02T17:49:11Z

Yep, everything ends up in the cache, it seems. At least I'm able to fetch stuff that shouldn't be referenced by any build's final closure. Unless the outputs are GCd at some point, they would eat up quite a lot of disk space and be essentially useless.

roberth · 2021-11-02T18:10:21Z

A potential solution is to override this setting in release.nix.
There's a bunch of wiring in between that needs to pass this bit of info along, but that could be done.
Invocations via pkgs.nixosTests would be unaffected by this, but it only works as long as hydra doesn't build package tests, which it should though: #136026
Maybe it needs to be a developer-only feature for now :(

flokli · 2021-11-02T18:21:10Z

Yes, there's no GC unfortunately. I'm not sure if we should keep all this in the binary cache. As long as there's no way to tell hydra to not upload that, I'm afraid we might need to revert the tests :-/

talyz · 2021-11-02T18:51:15Z

Well, currently, hydra fails to build the disk images since they larger than max_output_size, so there shouldn't be any need to revert in order to save space. If we want the tests to be able to build on hydra, however, something needs to change. To me, it feels like the proper solution would be to add an attribute which tells hydra not to bother caching the output - dontCache or something. I'm not sure how feasible it would be to do that, though.

It's all a bit ironic, since making this build better on hydra was one of my goals with this - the GitLab aarch64 tests were almost constantly timing out and the x86_64 test required ~20mins to run. At least it made the situation better on ofborg, where both aarch64 and x86_64 timed out previously.

vobloeb · 2022-02-23T21:14:49Z

9p read performance is not so bad, what woes in those Ruby=on-Rails cases are opendir and stat
NixOS might add some extension to 9p to improve it given read-only and same-timestamp properties of Nix Store.
Once upon a time there was a NixOS patch to 9p kernel module

vobloeb · 2022-02-23T21:19:36Z

As for virtiofs which requires root...
Well, nix build also requires root - either directly or with help of nix-daemon running under root.
So nix-daemon could setup virtiofs on request, just like does the daemon of libvirt.

roberth · 2022-07-12T09:39:23Z

Another possibility is to create an image not for the entire system, but only for the paths that matter for performance.
The /nix/store would then be an overlay of a block filesystem on top of 9p. This can reduce the size of the filesystem.
It also removes the need for putting the testScript dependencies in an image, solving the recursion problem more elegantly and without eval overhead.

When the filesystem is smaller, it may also be feasible to generate it at the last minute, in the test runner, so that it doesn't have be stored in the store. If the filesystem generation is quick enough, that'd be worthwhile. This can be done with mkfs.ext4 -d, which will be faster than make-disk-image.nix because it lacks virtualisation overhead and even faster than make-ext4-fs.nix.

Instead of virtualisation.useNixStoreImage = true; we'd say

virtualisation.copyStorePaths = [ config.services.gitlab.package ];

So our options for improving this are:

Optimize 9p directory operations in linux.
Add virtiofs to nix-daemon and test driver. This breaks in non-root sandbox, so 9p should also be supported. Deciding at eval time is bad UX. Deciding in the sandbox is impure and probably tricky to generate an fstab for it.
Put only part of the system closure in the image.

roberth · 2022-07-14T22:55:18Z

I've done some exploration with adding an overlay containing a subset of derivations, with squashfs and ext4.

Conclusion so far: a non-stored squashfs is a viable replacement for a stored ext4 image in the gitlab test, but not quite as efficient as what could be achieved by patching mkfs.ext4 -d.

Squashfs work in #181537.

Findings

a last-minute squashfs is feasible. Costs ~6s on my system for a 2G+ gitlab image with many .rb files
an overlay with an image containing only the gitlab packages is 40% slower than an image containing the entire store
ext4 has an option to load a directory into a filesystem image without virtualisation, but it doesn't support the selection of individual paths like gensquashfs and archivers do
squashfs has flags that optimize not just image creation but runtime performance. Compression gave a 25% slowdown.
a squashfs image containing the whole system toplevel seems to be equally fast as squashing only the gitlab packages
simulating an ext4 construction with mkfs.ext4 -d ./copied-paths (ie not counting the copying of paths to work around the missing optimization in mkfs.ext4 -d) seems 4× slower than mksquashfs 6s -> 23s. This is still ~100s faster than the overlay approach, or 25%. Only 8% slower than a saved ext4 image.

Open questions:

How about a complete squashfs image with all store paths? The ext4 numbers suggest that savings are possible.
How do these performance numbers hold up on the hardware of ofborg and hydra.nixos.org, or contributors' hardware? I think I'm willing to make assumptions though, as long as a typical contributor laptop benefits from the change.
How does all of this compare to possible performance optimizations in the 9p kernel module?
How does squashfs perform without an overlay? An optimization that applies when writableStore = false.

My data and some notes:

ext4 store image (this PR baseline)
277 seconds as reported by the test driver
+ ? seconds to build the image

9p host store + reversed overlay (ineffective) (probably close to baseline before this PR)
1198 seconds as reported by the test driver

9p host store + squashfs overlay
524 seconds as reported by the test driver
+ ? seconds to build the image

9p host store + uncompressed squashfs overlay
416 seconds as reported by the test driver
+ 6 seconds to build the image

9p host store + erofs overlay (default options)
386 seconds as reported by the test driver
+ 5 seconds to build the image

erofs (writableStore = false)
241 seconds as reported by the test driver
+ 7 seconds to build the image

9p host store + ext4 overlay
393 seconds as reported by the test driver
+ 23 seconds to build the image, excluding cp -r storepaths .

9p host store + uncompressed squashfs overlay with entire toplevel in the squashfs
410s including 5s for building the image

Hardware:

AMD Ryzen 9 3900 12-Core
ssd: SAMSUNG MZQLB1T9HAJR-00007
host store: btrfs

…n useNixStoreImage = true The involved test was nixosTests.nextcloud.basic21. It has a testScript that is strict in nodes.nextcloud.config.system.build.vm, in assertions about imagemagick being in the system closure. The recursion was introduced in 329a446 from NixOS#140792

talyz requested review from andir, flokli, rnhmjoj, ryantm, tfc and yu-re-ka October 6, 2021 18:51

github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 6, 2021

SuperSandro2000 reviewed Oct 6, 2021

View reviewed changes

nixos/lib/make-disk-image.nix Outdated Show resolved Hide resolved

SuperSandro2000 reviewed Oct 6, 2021

View reviewed changes

nixos/modules/virtualisation/qemu-vm.nix Outdated Show resolved Hide resolved

talyz force-pushed the qemu-vm-build-root-image branch 2 times, most recently from fe6016e to ce971b7 Compare October 6, 2021 23:45

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Oct 7, 2021

roberth reviewed Oct 7, 2021

View reviewed changes

nixos/modules/virtualisation/qemu-vm.nix Outdated Show resolved Hide resolved

talyz requested a review from dpausp October 9, 2021 09:05

talyz force-pushed the qemu-vm-build-root-image branch from 1bfc013 to 67a6de6 Compare October 14, 2021 14:39

roberth reviewed Oct 15, 2021

View reviewed changes

nixos/lib/testing-python.nix Outdated Show resolved Hide resolved

roberth reviewed Oct 15, 2021

View reviewed changes

nixos/lib/testing-python.nix Outdated Show resolved Hide resolved

talyz force-pushed the qemu-vm-build-root-image branch from 67a6de6 to 5d7977e Compare October 15, 2021 15:34

ofborg bot requested a review from roberth October 15, 2021 17:31

github-actions bot added the 8.has: documentation This PR adds or changes documentation label Oct 28, 2021

roberth reviewed Oct 28, 2021

View reviewed changes

roberth approved these changes Oct 28, 2021

View reviewed changes

ofborg bot requested a review from roberth October 28, 2021 10:38

yu-re-ka merged commit 33b3dae into NixOS:master Oct 28, 2021

talyz deleted the qemu-vm-build-root-image branch October 28, 2021 10:56

This was referenced Oct 31, 2021

NixOps test #142035

Merged

Evaluation error blocking nixos-unstable #143937

Closed

roberth mentioned this pull request Oct 31, 2021

nixosTest: Fix infinite recursion involving hasContext testScript whe… #144014

Merged

12 tasks

talyz mentioned this pull request Nov 1, 2021

nixos/testing-python: Avoid infinite recursion in node definitions #144110

Draft

12 tasks

roberth mentioned this pull request Jul 14, 2022

nixos/qemu-vm: Speed up tests with squashfs instead of stored ext4 image #181537

Closed

14 tasks

	target = "/nix/store";
	target = "/nix/store"; # should be storeDir, but is not supported in scripts yet

Uh oh!

Conversation

talyz commented Oct 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for this change

Things done

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roberth commented Oct 7, 2021

Uh oh!

talyz commented Oct 7, 2021

Uh oh!

mohe2015 commented Oct 7, 2021

Uh oh!

talyz commented Oct 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knedlsepp commented Oct 8, 2021

Uh oh!

andir commented Oct 8, 2021

Uh oh!

mohe2015 commented Oct 8, 2021

Uh oh!

talyz commented Oct 8, 2021

Uh oh!

talyz commented Oct 15, 2021

Uh oh!

roberth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

talyz commented Oct 28, 2021

Uh oh!

roberth Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

talyz Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

roberth Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

talyz Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

roberth commented Oct 31, 2021

Uh oh!

talyz commented Nov 1, 2021

Uh oh!

roberth commented Nov 1, 2021

Uh oh!

talyz commented Nov 1, 2021

Uh oh!

talyz commented Nov 2, 2021

Uh oh!

roberth commented Nov 2, 2021

Uh oh!

flokli commented Nov 2, 2021 via email

Uh oh!

talyz commented Nov 2, 2021

Uh oh!

vobloeb commented Feb 23, 2022

Uh oh!

vobloeb commented Feb 23, 2022

Uh oh!

roberth commented Jul 12, 2022

Uh oh!

roberth commented Jul 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

talyz commented Oct 6, 2021 •

edited

Loading

talyz commented Oct 7, 2021 •

edited

Loading

roberth commented Jul 14, 2022 •

edited

Loading