proposal: cmd/go: add flag option to control whether linked binary is put in build cache

[bradfitz]

Proposal Details

I propose that cmd/go have a new go test flag to control whether the linked test binary is written to the GOCACHE build cache.

Tiny example change: https://go-review.googlesource.com/c/go/+/739161

That change lets this,

$ go test -cachebinary -run=^TestFoo$ ./pkg/
$ go test -cachebinary -run=^TestBar$ ./pkg/

.... not have to link pkg.test on the second call.

From the docs there:

	-cachebinary
	    Cache the emphemeral test binary (the linker output) in the build
	    cache. This avoids re-linking the test binary if running the same
	    test binary repeatedly but with different flags, environment,
	    or other test inputs such that the test caching rules above
	    would not apply. On such case is Go test wrappers that shard tests
	    within a package across multiple machines, sharing a common build
	    cache with GOCACHEPROG.

I'm not wed to those exact docs, but that explains the background.

(I thought I remembered seeing this proposal once, but I can't find it, so maybe I'm remembering a conversation instead?)

/cc @golang/tools-team

Edit: renamed from -cachelink to -cachebinary

[gabyhelp]

Related Issues

• proposal: cmd/go: allow test binaries to identify as uncacheable #23799
• cmd/go: cache link output binaries in the build cache #69290 (closed)
• cmd/go: cache test results in go test #11193 (closed)
• cmd/go: linker repeatedly invoked for cached test results #49267
• proposal: cmd/go: add flag `go test -no-cache` #39056 (closed)
• cmd/go: speed up 'go run' by caching binaries #33468 (closed)
• proposal: testing: Add a way to disable cache for spesific tests #64462 (closed)
• cmd/go: coverage profile should be cached with tests #23565 (closed)
• proposal: cmd/go: add "go test -compile" option to just compile tests, not link or run them #46712 (closed)
• cmd/go: go test flag -skip should be cacheable #70692 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[mateusz834]

Curious: Why opt-in and not just always enabled?

[bradfitz]

@mateusz834 from the commit message on that change:

at the expense of writing more to the build cache.

... it blows up the size of the cache a bunch. And it's rarely needed.

[bradfitz]

Ah, #69290 from @matloob was the dup I couldn't find. (Thanks, @gabyhelp!)

But it doesn't look like it was proposed to make it an option like this proposal does, so I think this one still stands?

[IlyasYOY]

@bradfitz hey!

Your proposal looks interesting. I've been diving into the go test abyss recently. I have a few thoughts:

• Isn't the new flag the same for your use‑case as running go test ./package -o bin.test && ./bin.test -test.run "TestX" && ./bin.test -test.run "TestY"? I know that they are not the same (practically): cumbersome for multiple packages, go test provides a default 10 m timeout (and more, I guess).
• Will the CL above work without changing this code?

  go/src/cmd/go/internal/test/test.go

  Lines 1814 to 1828 in 985b0b3

  	case "-test.benchtime",
  	"-test.cpu",
  	"-test.list",
  	"-test.parallel",
  	"-test.run",
  	"-test.short",
  	"-test.skip",
  	"-test.timeout",
  	"-test.failfast",
  	"-test.v",
  	"-test.fullpath":
  	// These are cacheable.
  	// Note that this list is documented above,
  	// so if you add to this list, update the docs too.
  	cacheArgs = append(cacheArgs, arg)

  It looks like using the run flag changes the cache key before the link phase and invalidates the cache.
• This comment might also be useful:

  go/src/cmd/go/internal/work/buildid.go

  Lines 539 to 548 in 985b0b3

  	// TODO(matloob): If we end up caching all executables, the test executable will
  	// already be cached so building it won't do any work. But for now we won't
  	// cache all executables and instead only want to cache some:
  	// we only cache executables produced for 'go run' (and soon, for 'go tool').
  	//
  	// Special case for linking a test binary: if the only thing we
  	// want the binary for is to run the test, and the test result is cached,
  	// then to avoid the link step, report the link as up-to-date.
  	// We avoid the nested build ID problem in the previous special case
  	// by recording the test results in the cache under the action ID half.

  Maybe you meant it here: I remembered seeing this proposal once.

Building and reusing the binary looks like the solution. I hope that the thoughts above will help you.

[bradfitz]

@IlyasYOY,

Isn't the new flag the same for your use‑case as running go test ./package -o bin.test && ./bin.test -test.run "TestX" && ./bin.test

No. That way of running the test binary directly bypasses all test caching.

[gopherbot]

Change https://go.dev/cl/739161 mentions this issue: cmd/go: add 'go test -cachelink' boolean flag

[bradfitz]

Some concrete numbers.

Running our race tests before:

$ go clean -cache
$ go test -race -vet=off -count=1 ./...
...
real    8m14.365s
user    40m24.838s
sys     2m40.369s

$ du -h /home/bradfitz/.cache/go-build | tail -1
2.7G    /home/bradfitz/.cache/go-build

With -cachelink building all the binaries ahead of time in the background, before the main go test gets to them:

$ go clean -cache
$ go test -cachelink -p 256 -race -vet=off -count=1 ./... &
$ go test -cachelink -race -vet=off -count=1 ./...
...
real    5m2.745s
user    54m36.680s
sys     3m29.728s

$ du -h /home/bradfitz/.cache/go-build | tail -1
5.7G    /home/bradfitz/.cache/go-build

[ianlancetaylor]

Sorry to bikeshed, but I'm not wild about the name cachelink. We're not caching any links. How about cacheprog or something along those lines?

[seankhliao]

if this control is going to be exposed, maybe it should be a regular build flag instead of a test only flag?
it can have the current defaults of on for build, off for test, like I think how -buildvcs has different defaults too