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

go: support & use aggressive caching #4774

Merged
merged 3 commits into from
Aug 30, 2018
Merged

go: support & use aggressive caching #4774

merged 3 commits into from
Aug 30, 2018

Conversation

DomT4
Copy link
Member

@DomT4 DomT4 commented Aug 29, 2018

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

I hadn't planned to move on this so quickly but impressively core today had its first Go Module using PR, so this has become more useful than it would've been. Useful reading on the Modules system can be found via https://golang.org/doc/go1.11#modules.

GOCACHE has been a thing for a while but it hasn't really been especially beneficial to us, so I've ignored it, but with the Go Modules system it can be incredibly useful in reducing build times & thus CI burden. A lot of this is explained rather nicely here.

The upsides here for us are obvious:

  1. We do a lot of go-based builds, and the more things move over to the module system the more comprehensive that cache will become and we'll need to remotely fetch less dependencies, which massively speeds up build times even on high-speed connections.
  2. Module dependencies aren't limited per formula. If hugo fetches a dependency that is the same version as, for example, wiki also needs wiki won't bother to fetch that version remotely but instead will simply pluck it out of the cache.

If we look at hugo, because it's the only formula so far with Module support:

First build: built in 1 minute 41 seconds
Second build: built in 14 seconds.

It's worth noting perhaps that we can use just the module cache rather than the build cache + module cache combination, but doing so leads to a significant reduction in regained build time. A build under that system pops an average build time with hugo (after the first build) of ~70 seconds, which is only a 30 second saving on the first build. However, if we wanted to speed up builds whilst also not eating big chunks of disk space that is an option.

The downside of a build cache + module cache system is disk usage. go does not exactly eat up disk space sparingly, and the cache size from hugo alone is 258.8MB. That could be an issue on CIs where disk space is already a squeeze at times, but it's not much different to how we let the java_cache behave now. The major difference between java_cache and go_cache here I guess is that things that use go are much more regular visitors in the CI system.

All Module-supporting formulae need to do to use the shared cache is instead of deleting the GOPATH env we set everywhere replace it with this:

ENV["GOPATH"] = "#{HOMEBREW_CACHE}/go_cache"

DomT4 added 3 commits August 29, 2018 13:16
I hadn't planned to move on this so quickly but impressively core
today had its first Go Module using PR, so this has become more useful
than it would've been. Useful reading on the Modules system can be found
via https://golang.org/doc/go1.11#modules.

`GOCACHE` has been a thing for a while but it hasn't really been
especially beneficial to us, so I've ignored it, but with the Go Modules
system it can be incredibly useful in reducing build times & thus CI
burden. A lot of this is explained rather nicely here:
  https://groups.google.com/forum/#!msg/golang-dev/RjSj4bGSmsw/KMHhU8fmAwAJ

The upsides here for us are obvious:
1) We do a *lot* of go-based builds, and the more things move over to the
module system the more comprehensive that cache will become and we'll
need to remotely fetch less dependencies, which massively speeds up
build times even on high-speed connections.
2) Module dependencies aren't limited per formula. If `hugo` fetches a
dependency that is the same version as, for example, `wiki` also needs
`wiki` won't bother to fetch that version remotely but instead will
simply pluck it out of the cache.

If we look at `hugo`, because it's the only formula so far with Module
support:

First build: `built in 1 minute 41 seconds`
Second build: `built in 14 seconds`.

It's worth noting perhaps that we can use just the module cache rather
than the build cache + module cache combination, but doing so leads to
a significant reduction in regained build time. A build under that
system pops an average build time with `hugo` (after the first build)
of ~70 seconds, which is only a 30 second saving on the first build.
However, if we wanted to speed up builds whilst also not eating big chunks
of disk space that is an option.

The downside of a build cache + module cache system is disk usage.
`go` does not exactly eat up disk space sparingly, and the cache size
from `hugo` alone is 258.8MB. That could be an issue on CIs where disk
space is already a squeeze at times, but it's not much different to how
we let the `java_cache` behave now.

All Module-supporting formulae need to do to use the shared cache is
instead of deleting the `GOPATH` env we set everywhere replace it with
this:
```
ENV["GOPATH"] = "#{HOMEBREW_CACHE}/go_cache"
```
@ghost ghost assigned DomT4 Aug 29, 2018
@ghost ghost added the in progress Maintainers are working on this label Aug 29, 2018
@MikeMcQuaid
Copy link
Member

Nice work here @DomT4 👍

@DomT4 DomT4 merged commit baab9d8 into Homebrew:master Aug 30, 2018
@ghost ghost removed the in progress Maintainers are working on this label Aug 30, 2018
@DomT4 DomT4 deleted the GOCACHE branch August 30, 2018 03:45
@DomT4
Copy link
Member Author

DomT4 commented Aug 30, 2018

Thanks Mike! I'll try to keep an eye on how much this whacks CI on disk usage and review again if necessary.

There's a follow-up in #4778 to resolve an issue I missed initially, le sigh.

@commitay commitay mentioned this pull request Sep 4, 2018
6 tasks
@lock lock bot added the outdated PR was locked due to age label Sep 29, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Sep 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants