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

Use docker cache mounts for apt, pip and cargo #11106

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mjpieters
Copy link
Contributor

The cache mounts are cached using standard github actions cache when building in the CI pipeline.

Note that the build stage no longer contains the whole source tree, these are instead mounted into the build container when building to avoid invalidating cached build container layers.

@mjpieters mjpieters marked this pull request as ready for review January 30, 2025 18:07
@zanieb
Copy link
Member

zanieb commented Jan 30, 2025

Is this just for Docker image build performance?

@zanieb
Copy link
Member

zanieb commented Jan 30, 2025

See also, previous discussion at #3372

@mjpieters
Copy link
Contributor Author

Is this just for Docker image build performance?

Yes, both locally and CI workflows. I had completely missed the other PR 🤦 so thanks for the reference! I'll review the discussion and see if this needs closing or updating or if I change the merge target to that PR.

@zanieb
Copy link
Member

zanieb commented Jan 30, 2025

I think that one might have been a little more aggressive about refactoring. I'm more liable to just accept cache mounts. The big caveat there is probably captured by #3372 (comment) with the notable update that we're now using Depot runners in our org! So... we could set up Depot for Docker builds.

@mjpieters
Copy link
Contributor Author

mjpieters commented Jan 30, 2025

Yeah, the author of that pr clearly has a lot of rust-in-docker build knowledge, more than I have.

Bottom line is that this PR makes an incremental change with the aim of improving build time here in the GH pipeline. If you are moving the build to a different platform however then there is not much value in merging this one.

Dockerfile Outdated
Copy link
Collaborator

@samypr100 samypr100 Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, I would see #3372 for context as to why some of these changes haven't been done.

(edit: woops, hadn't seen this was already mentioned #11106 (comment))

@mjpieters

This comment was marked as outdated.

@mjpieters
Copy link
Contributor Author

I've now refactored the cache mounts to be simpler and (hopefully) more effective in the Github pipeline

All in-docker build tool caches now live in a single /buildkit-cache directory. This includes the global caches for cargo, rustup, pip, zig and cargo-zigbuild. This is keyed just on the Dockerfile hash, because these can trivially be re-used even if the Cargo.lock file changes.

The /root/target cache mount is no longer cached or restored in the GitHub pipeline, but it is still hugely helpful when building the docker container locally as it massively speeds up incremental builds in that case. Propagating the cache to the Github action cache was not effective however because when you have the exact same Cargo.lock, Cargo.toml and crates file tree then Github has also cached that specific Docker layer already, meaning that the whole build step is cached.

@mjpieters
Copy link
Contributor Author

mjpieters commented Jan 31, 2025

I forced a push and this does make the build faster, by about 90-120 seconds. Compare the times for the latest release:

I've since pushed another small update to add a rustup self update to use the cached version of rustup when available, instead of re-downloading and having to silence rustup from complaining about the cached cargo and rust toolchain.

The cache mounts are cached using standard github actions cache
when building in the CI pipeline.

Note that the build stage no longer contains the whole source tree,
these are instead mounted into the build container when building to
avoid invalidating cached build container layers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants