-
Notifications
You must be signed in to change notification settings - Fork 206
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: does the cache save us time? #502
Comments
1 and 2 are absolutely right... Only the top-level I'll put together a PR and benchmark it. Thanks for raising this, @warner |
This behaviour was fixed in actions/runner#268 but it has not yet been deployed to Github. I'm looking to find an ETA for when Github's Actions will deploy the fix. |
At @dtribble's request, I have disabled the cache in https://github.com/Agoric/agoric-sdk/commits/4cae3360d645a1ebe8dd707fa07e75b6a446e9c3 Leaving this issue open to revisit (and revert that commit) once Github Actions have deployed the fix described above. If it's past 2020-04-01, try doing that! |
Now the cache saves us time! |
If you look at the timings on https://github.com/Agoric/agoric-sdk/pull/489/checks?check_run_id=426471769 , it looks like we're spending 5 minutes hashing the
go.sum
andyarn.lock
files to conclude that nothing has changed and thus we don't need to save the cache.I'm guessing that the sheer number of files in go/pkg/mod and node_modules means anything which traverses
**/
is going to take forever, so we might want to change those to 1: only hash the single top-level yarn.lock (since there shouldn't be any others), 2: is there a similar top-level go.sum we could hash instead of finding all 67 of them?, 3: do we really need to invalidate these caches when those checksum files change anyways?The caches in question are only about 90MB each, so I'm not sure we're saving time overall there. Maybe we should just omit the cache actions from our
.github/actions/test-all-packages.yml
?The text was updated successfully, but these errors were encountered: