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

A way to output just a diff of ./target #76

Closed
dpc opened this issue Aug 14, 2022 · 13 comments · Fixed by #150
Closed

A way to output just a diff of ./target #76

dpc opened this issue Aug 14, 2022 · 13 comments · Fixed by #150

Comments

@dpc
Copy link
Contributor

dpc commented Aug 14, 2022

I was admiring the results of crane-based CI pipeline in Fedimint: https://github.com/fedimint/fedimint/actions/runs/2853966200 . After adding all the proper src filtering, when no source files were modified, nothing gets rebuild - not even tests are being re-run (since they've already passed successful, there's no point in re-running them).

Which is frankly super awesome, and thank you so much for working on crane! I can't overstate how well it works and how helpful you are. :)

However as you can see the build still takes ~2m, and it's all downloading stuff from cachix. There is something off there - as 2GB of stuff shouldn't take 2 minutes to download, IMO, but that's something I'm going to investigate on my own.

I started looking into why is it 2GB of data, and I realized that part of the reason is that build contains two target/ outputs:

Cachix: push
  /home/ubuntu/actions-runner/_work/_actions/cachix/cachix-action/v10/dist/main/push-paths.sh /home/ubuntu/.nix-profile/bin/cachix fedimint 
  compressing and pushing /nix/store/08mljabqrb8p11dl3y9ddqbakiss3mhl-source (108.28 KiB)
  compressing and pushing /nix/store/gw7pv5j6djw3rmkxvv9lj5az7l7xccfk-cargo-package-0.0.1 (489.87 MiB)
  compressing and pushing /nix/store/h1riypzjkqxc6qvw0m6f0kw7dvnrhai5-source (744.02 KiB)
  compressing and pushing /nix/store/acpnlx92cd7rgsm028l1g4mmf1575c4g-source (760.59 KiB)
  compressing and pushing /nix/store/9d3fwbq3bp0n03lrbs6msmacj2m9y7ml-cargo-package-0.0.1 (96.00 B)
  compressing and pushing /nix/store/glzdgkib6v3mf556vfhb7zy6ziqadk1l-cargo-package-0.0.1 (96.00 B)
  compressing and pushing /nix/store/9gmk58528m11yrra4miapcca0wv6ywnc-cargo-package-0.0.1 (420.72 MiB)
  compressing and pushing /nix/store/qdwmpys2zsr3klx1p92c730zw7jn0ali-cargo-package-0.0.1 (96.00 B)
  compressing and pushing /nix/store/qph9acr5h18gj51g1hv9d56ahngnxqym-cargo-package-0.0.1 (96.00 B)
  compressing and pushing /nix/store/sqaaj2ly4whg43iy05rikbcrc65j1azs-cargo-package-0.0.1 (96.00 B)
  compressing and pushing /nix/store/qf98j6391lrdmj9k0y0x2z94m99madl7-cargo-package-0.0.1 (96.00 B)

The 420MB one is the ./target after building the dependencies only, and 490M are the full workspace build.

And it made my realize - 80% of that 490M is redundant, isn't it? It is exactly same data that 420MB contains.

So I wonder - if crane allow somehow to store in $out only the files of ./target that are different from the input cargoArtifacts, then steps that need that output can just restore the first version, and then the diff, and get the same data, without storing anything twice.

The diffing itself might be slightly slower, but I think it will be more than made up for storage savings, in particular if network transfers are involved. And given that in the cloud CPU power is relatively cheap, yet storage is expensive it could be a big optimization.

This could possibly be all optional. The step producing ./target would have some doInstallCargoArtifactsDiffOnly = true;, and then downstream users would do cargoArtifacts = [ workspaceDeps workspaceFull ];. I wonder if it's possible to somehow write the reference to the base cargoArtifacts along the diff-only $out, so that the users don't even need to specify the [ base diff1 diff2 ... ] list, but I kind of doubt it (unless there's some Nix magic that I'm not aware of).

@ipetkov
Copy link
Owner

ipetkov commented Aug 15, 2022

There's definitely room for improvement here! The current implementation was easy to get up and running without making it too hard to juggle all the artifacts

Perhaps we can try something where we symlink the artifacts to earlier ones instead of copying them over completely after decompressing. Not sure if that will play nicely with cargo but it's worth the experiment

@dpc
Copy link
Contributor Author

dpc commented Aug 15, 2022

The current implementation was easy to get up and running without making it too hard to juggle all the artifacts

Yes. :)

I was thinking this will be particularly worthwhile for projects that have large workspaces and instead of all-deps + whole-workspace builds, want all-deps + app1 + app2 + app3.

Perhaps we can try something where we symlink the artifacts to earlier ones instead of copying them over completely after decompressing.

Smart. Worth giving a try. This works as long as cargo doesn't get spooked or tries to overwrite (now RO) existing files.

@ipetkov
Copy link
Owner

ipetkov commented Oct 29, 2022

@dpc I've opened #150 to use artifact symlinking by default which should bring down space usage in the Nix store when lots of derivations are chained!

Smart. Worth giving a try. This works as long as cargo doesn't get spooked or tries to overwrite (now RO) existing files.

Unfortunately it does look like rustc gets spooked if the artifacts are symlinks to read-only files (it doesn't try to unlink the files first) so we're still forced to fully copy the artifacts in the build directory, but at least we can dedup them in the Nix store

@dpc
Copy link
Contributor Author

dpc commented Oct 29, 2022

Sweet. I'll give it a try later today.

@dpc
Copy link
Contributor Author

dpc commented Oct 29, 2022

Well... I'm afraid I don't have good news. It seems like in the debug build that we are using now by default everything got significantly larger and takes longer.

The lack of compression seems like a biggest issue.

Before the dependencies would build to:

> ls -alh /nix/store/nq9h1q1d91c48sp7y39f2k0lza3ln7lb-workspace-deps-deps-0.0.1/target.tar.zst
-r--r--r-- 1 root root 648M Dec 31  1969 /nix/store/nq9h1q1d91c48sp7y39f2k0lza3ln7lb-workspace-deps-deps-0.0.1/target.tar.zst

and the workspace itself to:

> du -cksh result/
1.2G	result/
1.2G	total

Now the dependencies are:

> du -ckhs /nix/store/g7r4f97990p7m08qjqzqv74v0iql7660-workspace-deps-deps-0.0.1/target
3.8G	/nix/store/g7r4f97990p7m08qjqzqv74v0iql7660-workspace-deps-deps-0.0.1/target
3.8G	total

and the workspace build:

> du -cksh result/target/
3.1G	result/target/
3.1G	total

I've checked and du -cksh does not follow symlinks and with -L the sizes goes up to 6.xGB.

On top of it some parts seem to take a long, long while now, like:

workspace-build-build> symlinking duplicates in target to /nix/store/g7r4f97990p7m08qjqzqv74v0iql7660-workspace-deps-deps-0.0.1/target

and the fixupPhase generally (shrinking all the bins etc.)

For reference I'm trying nix build -L .#debug.workspaceBuild on https://github.com/fedimint/fedimint project.

@dpc
Copy link
Contributor Author

dpc commented Oct 29, 2022

Release build metrics

Release

Before:

Deps:

> du -cksh /nix/store/1lhnvkdxndbs0lzbxx3qisbn5d5k0ydv-workspace-deps-deps-0.0.1/target.tar.zst
476M	/nix/store/1lhnvkdxndbs0lzbxx3qisbn5d5k0ydv-workspace-deps-deps-0.0.1/target.tar.zst
476M	total

Workspace:

> du -cksh result/
547M	result/
547M	total

After:

Deps:

> du -cksh /nix/store/3f7l3fp0qchdsb615lpzjgjpppsnmy29-workspace-deps-deps-0.0.1/target
2.9G	/nix/store/3f7l3fp0qchdsb615lpzjgjpppsnmy29-workspace-deps-deps-0.0.1/target
2.9G	total

Workspace:

> du -cksh result/
522M	result/
522M	total

A bit better, but still rather bad. :)

@dpc
Copy link
Contributor Author

dpc commented Oct 29, 2022

@ipetkov Some thoughts:

Compression of the artifacts with zstd is too good to give up.

That probably pushes towards the "layers" approach, where instead of symlinking particular files, we would link to the "previous-layer.zstd" (possibly daisy chained).

I'm not sure what is being used for deduplication now, but it seems slow AFAICT (didn't meassure but looked like between 1 to 3 minutes).
Possibly the some approach where target.tgz is uncompressed and then file timestamps are used to find new files would be better?

@ipetkov ipetkov reopened this Oct 30, 2022
@ipetkov
Copy link
Owner

ipetkov commented Oct 30, 2022

I wonder if both the tarball and the zstd compression are finding ways to dedup common parts across all files. The current deduping strategy only dedups if the same file exists with the same contents as the previous build and that's pretty much it.

When I was thinking about using symlinking instead of compressing I kept rejecting the approach of combining the two together because compressing after deduping would hide the symlinks from Nix in a way that it won't be able to automatically track what outputs are chained to what.

Except I totally didn't consider the fact that we can drop a "manifest file" which contains the paths to any previously built artifacts. I think that may give us a best-of-both-worlds approach where we can first dedup files via symlink where we can, then pack and compress the results!

I'll work on implementing the idea above, but in the mean time, feel free to put installCargoArtifactsMode = "use-zstd"; to get the old behavior back

@dpc
Copy link
Contributor Author

dpc commented Oct 30, 2022

I wonder if both the tarball and the zstd compression

I'm confused. "tarball" is "zstd compressed". Aren't these two the same thing?

When I was thinking about using symlinking instead of compressing I kept rejecting the approach of combining the two together because compressing after deduping would hide the symlinks from Nix in a way that it won't be able to automatically track what outputs are chained to what.

I'm confused about everything here.

In my imagination the output of a workspace build would be:

target-prev -> /nix/store/...-packages-deps/ # symlink to the build artifacts of dependencies
target.zstd # new files

If another package were to build on top of this incrementally, crane would:

notice there's target-prev, see if target-prev/target-prev exists and recourse, extract target-prev/target.zstd, extract target.zstd to overwrite/add new files.

As for deduplication:

After extracting ./target, before running cargo recursively set a timestamp to some magic modification timestamp. After build step, copy over only files that don't have that magic modification timstamp.

@ipetkov
Copy link
Owner

ipetkov commented Oct 30, 2022

I'm confused. "tarball" is "zstd compressed". Aren't these two the same thing?

We do both here. tar knows how to combine a bunch of separate files into one big file. zstd knows how to make that resulting file smaller

I'm confused about everything here.

Heh, sorry just thinking out loud. Actually that idea I wrote up might not fully work since we don't have individual files to link to so it maybe needs more thought with a fresh head on my part 😉

If another package were to build on top of this incrementally, crane would:

notice there's target-prev, see if target-prev/target-prev exists and recourse, extract target-prev/target.zstd, extract target.zstd to overwrite/add new files.

That's an interesting idea! I'll do some experimentation

@dpc
Copy link
Contributor Author

dpc commented Sep 15, 2023

Thought: absent other ideas, a simple Rust program:

dir-diff <old-target> <new-target> > $out/target.zstd.tgz

that uses walkdir to compare two paths and outputs a zstd archive with new/changed paths would do. I know it's possible because I wrote a Rust program producing zstd tar archive recently.

The program could have deep vs mtime modes, and use mtime and stats optimizations even when in byte-for-byte (deep) comparison to make it fast, and zstd under the hood shouldn't have any overheads over using zstd binary, AFAICT.

@dpc dpc mentioned this issue Sep 17, 2023
4 tasks
@dpc
Copy link
Contributor Author

dpc commented Sep 17, 2023

We had a productive argument with @j-baker in #385 where we elaborated about pros&cons of symlinking files into ./target vs taring&compressing them with zstd.

@ipetkov
Copy link
Owner

ipetkov commented Nov 5, 2023

Implemented in #398!

@ipetkov ipetkov closed this as completed Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants