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

ci: Cache Cargo output on Github Actions #7768

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

Aaron1011
Copy link
Member

No description provided.

@Herschel
Copy link
Member

How does using the native GH cache action compare to https://github.com/marketplace/actions/rust-cache ?

@Aaron1011
Copy link
Member Author

I'm not sure - I'll test that out in a separate PR.

@Herschel
Copy link
Member

Herschel commented Aug 26, 2022

I ask because I remember there being an issue with not being able to clean our own artifacts from the target folder (rust-lang/cargo#5885), causing the cache to have to be re-uploaded on practically every build. Not 100% sure if this is still an issue with either cache or rust-cache methods.

@relrelb
Copy link
Contributor

relrelb commented Aug 26, 2022

Can you please prefix the commit message with ci: (or prepend it on merge using squash)?

@adrian17
Copy link
Collaborator

adrian17 commented Aug 26, 2022

FYI when using rust-cache, I had issues with some jobs still "ignoring the cache" - as in, they restore the cache, but the actual build still starts with all the dependencies from scratch.
See: https://github.com/adrian17/ruffle/runs/7819309280?check_suite_focus=true
Some of the jobs do clippy and test build fast, others in 5x the time due to dependencies.
I didn't figure this out, unfortunately. Not sure if this was related to rust-cache specifically, caches in general, or something wrong we did.

@relrelb relrelb changed the title Cache Cargo output on Github Actions ci: Cache Cargo output on Github Actions Aug 26, 2022
@adrian17
Copy link
Collaborator

Looks like indeed they try redownloading and rebuilding the crates :/ Even wasm-bindgen.

@Herschel
Copy link
Member

Herschel commented Aug 30, 2022

Ok, took some time to look at the behavior of GitHub Actions here:

  • All of the jobs on the same OS were using the same cache, despite being matrixed for different targets/Rust versions.
  • GitHub Actions caches are immutable once created.
  • This means on the initial cache, one job would "win" and create the cache. This specific job would be nice and cached on future runs.
  • But the other jobs on that same OS would have none of their cached dependencies, causing them to churn on various dependencies.

(because caches are immutable, the "cache always growing" issue I mentioned above doesn't apply, unless we explicitly make a new cache each build).

So I made these changes:

  • Add web, desktop, and matrix.rust_version to the cache key, so there are separate caches for each of these builds.
  • Skip the install wasm-bindgen step if the cache was hit.

Here's CI completing in <10 minutes:
https://github.com/Herschel/ruffle/runs/8083688920?check_suite_focus=true

I'm sure there are some improvements here -- for example, probably .cargo only needs to be keyed by OS, and target can be in a separate cache keyed by Rust version -- but fiddling with CI is probably my least favorite thing. 😆

@Herschel
Copy link
Member

Herschel commented Aug 30, 2022

Looked over rust-cache a bit, and it's by default keyed with rustc version as I described above. I believe the reason it was acting up in @adrian17's examples was that it was also keys by job_id, and both our desktop and web workflows have a job_id of build, causing the same cache to be used in both desktop and web builds. Things are working good now that I changed this to an explicit desktop/web key.

https://github.com/Herschel/ruffle/runs/8085728266?check_suite_focus=true

rust-cache has some smarts to clean the cache of stale artifacts etc., so let's go with that.

Co-authored-by: Mike Welsh <mwelsh@gmail.com>
@Herschel Herschel merged commit 3417dce into ruffle-rs:master Aug 30, 2022
@Aaron1011 Aaron1011 deleted the gha-cache branch August 30, 2022 14:35
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.

4 participants