Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos: compress make-ext4-fs with zstd #75592

Merged
merged 1 commit into from Dec 14, 2019

Conversation

@lovesegfault
Copy link
Contributor

@lovesegfault lovesegfault commented Dec 13, 2019

Motivation for this change

The AArch64 build has been failing for a few days with the output of it's ext4 image being >2.0GB. This compresses it with bz2, thus solving the problem.

I'll add that currently we compress all these large images with bz2, which is very slow, I'd like to suggest a change to zstd or lzo. This would be specially meaningful in architectures where usually there are more cores at the expense of clock (since compression/decompression is single threaded.) In particular ARM families would benefit, with no downside to other arches. I am preparing this in another branch, but would like to see this one merged first.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @samueldr @andir @grahamc @cleverca22

@lovesegfault
Copy link
Contributor Author

@lovesegfault lovesegfault commented Dec 13, 2019

Difference between bzip2 and zstd

❯ time bzip2 -k nixos-sd-image-20.03pre204199.3140fa89c51-aarch64-linux.img
bzip2 -k nixos-sd-image-20.03pre204199.3140fa89c51-aarch64-linux.img  145.93s user 0.44s system 99% cpu 2:26.39 total
❯ time zstd nixos-sd-image-20.03pre204199.3140fa89c51-aarch64-linux.img
nixos-sd-image-20.03pre204199.3140fa89c51-aarch64-linux.img : 26.99%   (2189545472 => 591009441 bytes, nixos-sd-image-20.03pre204199.3140fa89c51-aarch64-linux.img.zst)
zstd nixos-sd-image-20.03pre204199.3140fa89c51-aarch64-linux.img  6.50s user 0.33s system 110% cpu 6.196 total
@lovesegfault
Copy link
Contributor Author

@lovesegfault lovesegfault commented Dec 13, 2019

I've done the work to use zstd in another branch and the difference is staggering.

Copy link
Member

@samueldr samueldr left a comment

As it is, this breaks the API of make-ext4-fs.nix.

I do not know what is our guarantee with internal~ish builders like those.

This could be fixed by adding a compressed ? false param to the builder. the main difference would be that bzip2 invocation is not used, but rather a cp.

nixos/lib/make-ext4-fs.nix Outdated Show resolved Hide resolved
nixos/lib/make-ext4-fs.nix Outdated Show resolved Hide resolved
nixos/lib/make-ext4-fs.nix Outdated Show resolved Hide resolved
nixos/lib/make-ext4-fs.nix Outdated Show resolved Hide resolved
nixos/modules/installer/cd-dvd/sd-image.nix Outdated Show resolved Hide resolved
Copy link
Member

@samueldr samueldr left a comment

(Ugh, forgot to tick request changes)

@samueldr
Copy link
Member

@samueldr samueldr commented Dec 13, 2019

I don't think that the following is an issue with which we should concern ourselves right now, but to be kept in mind for the eventual rewrite.

Writing a temporary file is costly. It could result in failing builds if the $NIX_BUILD_TOP directory ends up in a tmpfs, and memory is in contention. A future approach would better be served if it could avoid copying the files and rather directly compressing / streaming in place.

For the second part (streaming into the final image), it would need metadata written in $out (so $out would not be directly an image file) that tells the image builder what is the size of the image. For the first part, (compression) I guess it depends on what compresses the image file. Are there ways that it can be done in-place? Though, if it's writing in a $out directory, rather than a $out file, the temp file could be in $out and removed as needed if this is a concern.

A final strategy for the future could be to merge multiple snippets into one big honking derivation that does all the work. The main drawback would be that the derivation is less flexible, and takes more time to build. A well architected solution could possibly do this with an option.

@lovesegfault lovesegfault force-pushed the lovesegfault:ext4-fs-compression branch from d378910 to 21d7243 Dec 13, 2019
@lovesegfault lovesegfault requested a review from samueldr Dec 13, 2019
@lovesegfault lovesegfault changed the title nixos/lib: compress make-ext4-fs with bzip2 nixos/lib: compress make-ext4-fs with zstd Dec 13, 2019
@lovesegfault lovesegfault force-pushed the lovesegfault:ext4-fs-compression branch from 21d7243 to 5f5a7b1 Dec 13, 2019
@lovesegfault lovesegfault changed the title nixos/lib: compress make-ext4-fs with zstd nixos: compress make-ext4-fs with zstd Dec 13, 2019
@lovesegfault lovesegfault force-pushed the lovesegfault:ext4-fs-compression branch from e41ae61 to 3891832 Dec 13, 2019
nixos/modules/installer/cd-dvd/sd-image.nix Outdated Show resolved Hide resolved
nixos/lib/make-ext4-fs.nix Outdated Show resolved Hide resolved
@lovesegfault lovesegfault force-pushed the lovesegfault:ext4-fs-compression branch 2 times, most recently from 73ecb62 to 8fdf1a1 Dec 13, 2019
@lovesegfault lovesegfault requested a review from samueldr Dec 13, 2019
@lovesegfault lovesegfault force-pushed the lovesegfault:ext4-fs-compression branch from 8fdf1a1 to 70c5a78 Dec 14, 2019
@samueldr samueldr merged commit f8ab1a9 into NixOS:master Dec 14, 2019
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@lovesegfault lovesegfault deleted the lovesegfault:ext4-fs-compression branch Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.