Build output caching does not key off the current module's source?

[nfi-hashicorp]

IIUC, a job with cache enabled goes roughly like this (with defaults):

1. (setup-go) restore: download and extract cache blob, keyed by (among other things) go.sum
2. (user step) build something
3. (setup-go) save: if cache has key, skip. Otherwise, make a cache blob containing the "module" cache (i.e. source code) the build cache at ~/.cache/go-build/ and upload it with that key.

The key is:

const primaryKey = `setup-go-${platform}-${linuxVersion}go-${versionSpec}-${fileHash}`;

--- https://github.com/actions/setup-go/blob/main/src/cache-restore.ts#L34

But that doesn't include any inputs from the current module's source (other than go.sum)? So really, we're just caching the build outputs of the dependencies?

(Or to be more precise, we bundle all of ~/.cache/go-build, but only when go.sum changes. If we did a build of the current module, then the build cache will contain its build objects. But it will never get updated if only the source of the current module changed.)

It's unclear if this is intended or an oversight. The README.md definitely isn't explicit that the current module's source is not considered WRT to the cache key and whether a save needs to happen.

Also, I haven't really thought about it, but I'm not sure how cache test results play into this.

[dsame]

Hello, the cached directories by default are the those that are outputs of go env GOMODCACHE and go env GOCACHE

The cache invalidates if any of the files in the input cache-dependency-path are updated, by default it is only go.sum

To invalidate the cache on the changes to files other than go.sum the cache-dependency-path should be set explicitly (glob patterns are allowed).

Did the answer help?

[nfi-hashicorp]

Thanks, yes, that confirms what I've gathered from the docs/source.

One other problem I just realized is that the cache key also misses dynamic things like build tags, linker flags, -trimpath, maybe some env vars, etc. Also it seems like platform in the cache key is just the OS of the runner, so if you're doing cross-platform builds, that could probably cause a false cache hit. For any reasonably advanced build, a cache-dependency-path glob like src/** is probably not adequate.

Given that behavior, I'd say there are three actual problems:

Docs should say the caching is for dependencies, not the current module (by default)

And it doesn't really give guidance for caching the current module (like you'd just given me). Even if we added that, I'd say there are a lot of caveats; I as a user probably wouldn't feel confident configuring it, and I bet I'm not alone.

The saved cache may include objects for the current module anyway

This bloats the size of the cache bundle, meaning longer download and extraction times. Not generally a huge problem in the grand scheme of things, but for example, our cache download and extract currently adds about 10 seconds, and due to problems mentioned above, largely doesn't help with build times later on.

I don't have a good suggestion for this. AFAICT there's no good way to even ask Go which cache objects belong to which packages.

Current cache key is probably inadequate

Even just for dependencies, I think the current key is probably inadequate for all but the simplest builds. I'm struggling to find a reference that lists all the various inputs to a build that could invalidate the cache, but I bet they're a lot.

I'm not sure how to even begin to address this. You'd basically have to implement a large chunk of the internal go build caching logic, not fun.

Just how broken is this?

I would say that if you're doing anything much more complicated than vanilla go build ., reusing the build cache is probably not helping you any, and given a big enough build, is probably hurting to some degree. IMO this should be called out in the docs at least.

And I'm not sure setup-go could help with this much, needs better tooling from Go itself, but it'd be nice if we could profile cache hits at a per-object level to see if it's worth doing. I'm pretty sure with consul we're doing cross-platform builds and have the cache turned on, which is probably 100% miss for builds ATM.

PS I don't want it to sound like I'm ragging on the team for their efforts in implementing this; it's hard! Caching builds is far more complicated than one would think.

[dsame]

Hi @nfi-hashicorp, you're more than welcome to describe the missing features. I understand your requirements and the short answer is: the actions/cache is for you.

Reasoning
The primary task for setup-go is to work out of the box as much as possible, and to automate what can be automated. Any manual fine-tuning of caching is generally outside the scope of these actions, and is better implemented with full-power actions/cache action.

For example, "current key is probably inadequate for all but the simplest builds" - this is meant to be a compromise for an average use case: caching rarely changing third-party dependencies.

Trying to make it more specific, or to include more than 3rd party dependencies, leads to the cache being invalidated too often (and becoming useless), or growing rapidly and hitting the cache size limit.

"The saved cache may still contain items for the current module" - yes, but the structure of the project is not always the same, and it is nearly impossible to determine which folders can be cached without resetting the cache on every build.

"Docs should say the caching is for dependencies" - this may or may not be true depending on the value of cache-dependency-path, but the cached folders are listed in the logs.

Summary.
For a productive discussion, I would advise you to take a look at actions/cache and try to solve the specific real-life problem you have with it. Next, knowing the problem and the solution will be a good starting point to discuss whether it can and should be implemented as a built-in, automated feature of `setup-go'.

[nfi-hashicorp]

I understand your requirements and the short answer is: the actions/cache is for you.

Yeah, possibly. But that means coming up with a cache key that incorporates every input. Possible, maybe, but certainly untenable in the general case.

The primary task for setup-go is to work out of the box as much as possible, and to automate what can be automated.

Personally I think caching should be off by default. It is wasteful in all but the simplest build scenario. It's probably 100% wasted in a cross-platform build scenario, which I would be willing to bet is not uncommon. Even in the simplest case, I don't know how much of a speed up caching would bring. Do you have data about this?

I think one of the bigger UX problems is it's hard to measure the cache hit rate of the actual Go build. Sure, we can see if the setup-go cache key hits, but not if Go actually uses any of its contents.

[dsame]

Hello @nfi-hashicorp ,

With actions/cache it is possible to create any keys using the context and expressions and cache any paths moreover with restore-keys input it is possible to populate invalidated cache with data from the previous build.

Caching was off by default for a long time until few months ago the investigation had been made and according to its results it was decided to turn it on by default.

I'd be extremely grateful if you suggested the build configurations you think not cached in the optimal way an we would improve the action.

[nfi-hashicorp]

In this build, I have two parallel flows, one that builds an AMD64 build twice, and one that builds an ARM64 build twice. I deleted all caches for this repo before starting.

You would expect to see the first build in each flow be slow, since it's building from no cache, then the second one should be fast. But that's not what you see

Only the AMD64 one gets a speed boost, because the cache key doesn't include GOARCH.

Indeed, it's slower than a clean build, because it spent almost 30 seconds downloading a 638 MB cache for the wrong arch.

(Also, I notice that since the two flows collide on the same cache key, the second flow ends up failing to save the cache tarball, wasting 30s making a tarball.)

As another example, this build does something similar, but instead of switching arches, it switches on -trimpath.

Similar problem to the first example, the second -trimpath build is slow, even though you would have expected the results from the first build to be cached. They aren't because the cache key doesn't know about -trimpath. And again, it wasted 30s downloading and extracting an irrelevant cache.

The point

My point isn't that these two inputs need to be added to the cache key, it's that the cache key has a lot more inputs than these and it would be a lot of work to enumerate (let alone capture!) them. And a cache miss can be quite expensive, just to download the tarball. So yeah, I'd be willing to bet that in most non-trivial go builds, it is actually a detriment, but I'd need to see the data.

(Sidenote: Personally I think the Github action cache approach of uploading tarballs is too naive for most non-trivial builds.)

[nfi-hashicorp]

I should note that the above really only applies to the build cache (GOCACHE). The logic for the mod cache (GOMODCACHE) is probably ok. However, I can see no good way to tell setup-go to ignore the build cache.

[dsame]

Thank you @nfi-hashicorp , we are starting to investigate the case

[nfi-hashicorp]

Thanks! I appreciate all the hard work, LMK if I can help

[dsame]

Hello @nfi-hashicorp , it seems i am getting the idea and tying to reproduce the problem, but it would be helpful to see the builds you've mentioned above. I can not access them now - aren't they are in the private repository?