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 golangci-lint cache and Go build cache between runs on CI. #5160

Merged
merged 12 commits into from Jun 8, 2023

Conversation

charleskorn
Copy link
Contributor

@charleskorn charleskorn commented Jun 5, 2023

What this PR does

This PR introduces caching for golangci-lint's cache and the Go build cache to the CI build.

This improves the performance of each of the affected jobs if a cache hit occurs:

  • the build job is around 50% faster
  • the linting job is around 60% faster
  • the unit test job is around 30% faster
  • the integration test jobs vary, but most are ~90s faster

Overall, this improves CI run time by around 4 minutes, which is roughly 20%. For example, compare this run where no cache hits occur with this run where all caches are hit.

This uses GitHub Actions' built-in caching support, and so has the cross-branch caching protections described here.

Which issue(s) this PR fixes or relates to

Relates to #5159

Checklist

  • [n/a] Tests updated
  • [n/a] Documentation added
  • [n/a] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@charleskorn charleskorn changed the title Cache golangci-lint cache and Go build cache between linting runs on CI. Cache golangci-lint cache and Go build cache between runs on CI. Jun 5, 2023
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comments.

.github/workflows/test-build-deploy.yml Outdated Show resolved Hide resolved
.github/workflows/test-build-deploy.yml Show resolved Hide resolved
.github/workflows/test-build-deploy.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comments.

Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comments. I see GitHub still submits double reviews :/

Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also wondering if we shouldn't be caching $GOMODCACHE.

.github/workflows/test-build-deploy.yml Show resolved Hide resolved
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also wondering if we shouldn't be caching $GOMODCACHE. GitHub double review submit 🤦‍♂️

@aknuds1 aknuds1 self-requested a review June 6, 2023 07:25
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although I guess we can drop the debugging steps by now? Is the PR otherwise all done, considering it's still a draft?

@charleskorn
Copy link
Contributor Author

LGTM, although I guess we can drop the debugging steps by now? Is the PR otherwise all done, considering it's still a draft?

Yep, I need to drop the debugging steps, fix the double-caching from setup-go and add caching for GOMODCACHE to the linting job, as that's the only job that downloads modules.

Once I've made those changes, I'll mark this as ready for review.

@charleskorn charleskorn marked this pull request as ready for review June 7, 2023 00:05
@charleskorn charleskorn requested a review from a team as a code owner June 7, 2023 00:05
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@aknuds1 aknuds1 added the chore label Jun 7, 2023
@charleskorn charleskorn enabled auto-merge (squash) June 8, 2023 00:50
@charleskorn charleskorn merged commit 164bc7b into main Jun 8, 2023
24 checks passed
@charleskorn charleskorn deleted the charleskorn/build-caching branch June 8, 2023 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants