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 <kolyshkin@gmail.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
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č <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac
left a comment
There was a problem hiding this comment.
@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.
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.
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/GOARCHis maybe sufficient for go_build_cache (and, I guess, go_modules cache doesn't need it).
There was a problem hiding this comment.
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.
| - go version | ||
| - grep GOLANGCI_LINT_VERSION Makefile | head -1 | ||
| script: | | ||
| git remote update |
There was a problem hiding this comment.
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.
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.
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.
| - grep GOLANGCI_LINT_VERSION Makefile | head -1 | ||
| script: | | ||
| git remote update | ||
| make tools |
There was a problem hiding this comment.
All of this could be cached, or maybe built into the VM image.
There was a problem hiding this comment.
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č <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
Hi, and thank you for your contribution! We’ve recently migrated this repository into a new monorepo: containers/container-libs along with other repositories As part of this migration, this repository is no longer accepting new Pull-Requests and therefore this Pull-Request is being closed. Thank you very much for your contribution. We would appreciate your continued help in migrating this PR to the new container-libs repository. Please let us know if you are facing any issues. You can read more about the migration and the reasoning behind it in our blog post: Upcoming migration of three containers repositories to monorepo. Thanks again for your work and for supporting the containers ecosystem! |
No description provided.