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

Cache dependency crates #1094

Closed
wants to merge 2 commits into from

Conversation

Blisto91
Copy link

@Blisto91 Blisto91 commented Jul 16, 2020

This PR picks up from #1086.
Credit to @justinmoon for inspiring me to look into github caching and the workings of cargo. Learned a lot 😁

This adds caching of the dependency crates and crates.io index that are currently downloaded each time a job starts.
A separate update-deps job have been added where the crates will be cached and shared between the rest of the jobs.
If the dependencies change then the cache will be updated.

This has the extra benefit that we ensure each job in a workflow use the exact same dependency versions even if an update got pushed while the CI was setting up the different jobs.

The cache hit might miss on windows currently since it uses a different compression algorithm due to a bug. Will hopefully get fixed on it's own.

Since we currently don't cache the build target we can also get a small compile speed boost by setting CARGO_INCREMENTAL to 0.
Caching of the build target will be a separate PR since it is a bit more complicated to do properly with the large build sizes and the need to clean old build artifacts.

The code is heavily inspired from the Testing a library example in this PR. So also credit to @mzabaluev
Tho i have decided against having a separate cache for the crates.io index as shown in the example.
I don't think the double digit savings in seconds by not downloading the full index each workflow is worth the extra lines of code it would add.

Adds caching of the dependency crates Cargo downloads each time it would do a first compile on a new platform. Includes crates.io index.

This also ensures the exact same crate versions are used in each job since cargo.lock is only generated once per workflow.

Since we don't currently cache the target/ directory we also set CARGO_INCREMENTAL to 0.
This gives a small compile speed boost.
@Blisto91
Copy link
Author

Blisto91 commented Jul 16, 2020

Investigating why clippy is failing. Didn't during testing.

Edit:
New lint from a clippy update released today. Under the Rust 1.45 section

@Blisto91
Copy link
Author

Blisto91 commented Jul 18, 2020

I found during some testing that it might still download some crates in the main jobs. But it seems to only happen on windows.
Not sure what is happening since the fetch command in update-deps should fetch for all targets.
Investigating.

Edit:
Added a Temporary workaround for the cache miss on Windows because of the different compression algorithm.
We now generate a cache both on Ubuntu and Windows.
When Zstd gets fixed on windows then update-deps can be run just on Ubuntu.

Also we now update to latest rust before fetching deps.

Temporary workaround for cache miss on Windows because of the different compression algorithm.
When Zstd gets fixed on windows this can be run just on ubuntu.

Also we now update to latest rust before fetching deps.
@Blisto91
Copy link
Author

Having second thoughts on caching only the download dependency crates this way.
Looking more into the numbers i think this is not worth it alone currently, because the savings are relatively small.
Combined with the fact that Github cache restore times seems to fluctuate quite a lot as of this moment.
Sometimes it takes seconds to restore a cache and other times it can take minutes even for the same cache files.

I do think the approach of generating Cargo.lock in one job and then sharing that in the other jobs has some value tho.
I might open a new PR for caching the build target since that is where the CI spends the most of it's time. Some real gains in time can be achieved there.
But i'm currently having problems satisfying both the Github cache size limits while still being able successfully build from the cache on all targets. At least on stable Rust
Looking into it still tho so will try to return with something better 😁

@Blisto91 Blisto91 closed this Jul 19, 2020
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.

1 participant