-
Notifications
You must be signed in to change notification settings - Fork 397
Testing Cirrus-CI caching #2837
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
base: main
Are you sure you want to change the base?
Conversation
This shaves off some time (40 seconds, ~15%) from a validate job (unless either golangci-lint or go version changes). The cache size is pretty small (currently 686Kb compressed). Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Substantially, this notes a FIXME Also, this is a new commit and should allow us to compare timing with cache present. Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kolyshkin FYI: caching GOMODCACHE
(like https://cirrus-ci.org/examples/#go suggests) helps more (validate_task
goes from maybe 3:13 to 3:10), and caching GOCACHE
(like https://github.com/actions/setup-go does) can be a much bigger deal (cross_task
from 3:24 to 0:32).
.cirrus.yml
Outdated
go_modules_cache: &go_modules_cache | ||
fingerprint_script: cat go.sum | ||
folder: $GOPATH/pkg/mod | ||
go_build_cache: &go_build_cache | ||
fingerprint_script: cat go.sum | ||
folder: $GOCACHE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these need more to fingerprint. Compare https://github.com/actions/setup-go/blob/dca8468d37b6d090cde2c7b97b738a37134f5ffb/src/cache-restore.ts#L35 in GitHub.
And none of that triggers a cache rebuild on code changes, so eventually just changing code in the project will make the caches obsolete. Maybe the weekly Renovate PRs are enough to trigger go.sum
changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these need more to fingerprint.
In fact, go version
already has GOOS/GOARCH (in addition to the version itself), so having something like
fingerprint_script:
- cat go.sum # or go.mod?
- go version # version + GOOS/GOARCH
is maybe sufficient for go_build_cache (and, I guess, go_modules cache doesn't need it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
On go.sum
vs. go.mod
, I think the two should change in exactly the same commits — but, conceptually, go.sum
would be updated on tag changes, while go.mod
might not (e.g. after reverting to a previous dependency version, we might hypothetically see exactly the same go.mod
but different go.sum
exposing that a tag was moved).
Ultimately, none of this should matter for correctness, hopefully the Go caching mechanism is capturing all relevant inputs in its cache design.
reupload_on_changes: true | ||
fingerprint_script: | ||
- go version | ||
- grep GOLANGCI_LINT_VERSION Makefile | head -1 | ||
script: | | ||
git remote update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: This seems mostly unnecessary, we just want the destination branch.
.cirrus.yml
Outdated
go_modules_cache: &go_modules_cache | ||
fingerprint_script: cat go.sum | ||
folder: $GOPATH/pkg/mod | ||
go_build_cache: &go_build_cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very big deal for cross_task
. I would have expected this to also make a big difference for validate_task
, but it doesn’t?!
A hypothesis to examine: would using a separate cache for the two tasks help? The current test uses a cross
-created cache for validate
; maybe the two differ enough in environment (some extra build tags??) that the caches don’t match?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes: a separate build cache brings validate_task
down to 0:42 when nothing in the code changed (vs. 4:19 with no caching at all, 3:13 with lint caches, and 3:10 with module cache).
That is definitely worth doing.
reupload_on_changes: true | ||
fingerprint_script: | ||
- go version | ||
- grep GOLANGCI_LINT_VERSION Makefile | head -1 | ||
script: | | ||
git remote update | ||
make tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this could be cached, or maybe built into the VM image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separating the caches between validate_task
and cross_task
helps at least in the sense that we don’t download git-validation
and its dependencies (and hopefully the compilation is also avoided).
Not the best we can do, but getting closer to the point of diminishing returns.
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
No description provided.