cmd/go: cache hit takes too long on a test that opens many files

[ijt]

What version of Go are you using (go version)?

go1.10.2

Does this issue reproduce with the latest release?

Yes.

ijt$ time go1.11beta1 test .                                                                                                                                                                                                                                                                                                 
ok      github.com/google/go-cloud/wire/internal/wire   38.037s

real    1m10.576s
user    1m22.899s
sys     0m52.377s
ijt$ time go1.11beta1 test .
ok      github.com/google/go-cloud/wire/internal/wire   (cached)

real    0m30.822s
user    0m11.384s
sys     0m20.135s

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/issactrotts/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/issactrotts/gopath"
GORACE=""
GOROOT="/Users/issactrotts/homebrew/Cellar/go/1.10.2/libexec"
GOTMPDIR=""
GOTOOLDIR="/Users/issactrotts/homebrew/Cellar/go/1.10.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/9z/3vhqmj_j18v6vsk1q2jr486r001n0v/T/go-build330802734=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

m=github.com/google/go-cloud
go get $m
cd $(go list -f '{{.Dir}}' $m)
cd wire/internal/wire
# Run tests once.
time go test -v .
# Run tests again. This time the result is cached, but it still takes over 30s on a 2015 Macbook pro laptop.
time go test -v .

What did you expect to see?

The second test run should have taken under 1s since nothing changed.

What did you see instead?

It took over 30s.

@zombiezen suggested that the slowness arises from the wire tests opening many packages under $GOROOT for type checking, making go test spend a lot of time checking their hashes when deciding whether there is a cache hit.

[mvdan]

Perhaps related to https://github.com/golang/go/issues/26562.

[ijt]

I ran it again like this

GODEBUG=gocachehash=1 go test .

and the output seems to say that the same hashes are being recomputed many times.

...
HASH[getenv]
HASH[getenv]: "go1.10.3"
HASH[getenv]: "\x00"
HASH[getenv]: 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471
HASH[testInputs]: "env TMPDIR 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471\n"
HASH[getenv]
HASH[getenv]: "go1.10.3"
HASH[getenv]: "\x00"
HASH[getenv]: 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471
HASH[testInputs]: "env TMPDIR 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471\n"
HASH[getenv]
HASH[getenv]: "go1.10.3"
HASH[getenv]: "\x00"
HASH[getenv]: 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471
HASH[testInputs]: "env TMPDIR 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471\n"
HASH[getenv]
HASH[getenv]: "go1.10.3"
HASH[getenv]: "\x00"
HASH[getenv]: 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471
HASH[testInputs]: "env TMPDIR 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471\n"
HASH[getenv]
HASH[getenv]: "go1.10.3"
HASH[getenv]: "\x00"
HASH[getenv]: 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471
HASH[testInputs]: "env TMPDIR 213baadbc0781c1584c3a372b0c954145a41421235ba99e327ec0dd4f2326471\n"
HASH[testInputs]: 5650a478a2695895ea5dad0d10e315fe75f5d25b69bd3e0a20012a2c5adf2548
HASH subkey 422e11c5f77c95dbb5a70bf9d3f8cb86c84e52ac80a14030b5622b2fed39ed70 "inputs:5650a478a2695895ea5dad0d10e315fe75f5d25b69bd3e0a20012a2c5adf2548" = e3e7ae218e3b146458d106618002b8e7b2dbad22e5e8fd295d858549b0ac9ec0
ok      github.com/google/go-cloud/wire/internal/wire   (cached)

It looks like the check could be sped up a lot by only computing the hashes once.

[gopherbot]

Change https://golang.org/cl/127155 mentions this issue: cmd/go: check test cache more efficiently

[ijt]

I instrumented vgo test and got this output from go tool pprof:

It shows that the inDir function takes up most of the time.

[ijt]

That lends support to the idea raised in issue #26562 that the slowdown was introduced in https://github.com/golang/go/commit/37d56279c87818b496e5717bddd1f7c43bfa743d.

[ijt]

Simplifying inDir to no longer call EvalSymlinks fixes the speed problem.

diff --git a/vendor/cmd/go/internal/test/test.go b/vendor/cmd/go/internal/test/test.go
index aff5ff2..02fc6cc 100644
--- a/vendor/cmd/go/internal/test/test.go
+++ b/vendor/cmd/go/internal/test/test.go
@@ -1429,15 +1429,7 @@ func computeTestInputsID(a *work.Action, testlog []byte) (cache.ActionID, error)
 }

 func inDir(path, dir string) bool {
-       if str.HasFilePathPrefix(path, dir) {
-               return true
-       }
-       xpath, err1 := filepath.EvalSymlinks(path)
-       xdir, err2 := filepath.EvalSymlinks(dir)
-       if err1 == nil && err2 == nil && str.HasFilePathPrefix(xpath, xdir) {
-               return true
-       }
-       return false
+       return str.HasFilePathPrefix(path, dir)
 }

The question is how to do it without giving up correctness.

[mvdan]

The symlink issue is being fixed in #26562 already.

[gopherbot]

Change https://golang.org/cl/127635 mentions this issue: cmd/go: no longer eval symlinks in inDir

[ijt]

Closing this as a duplicate of #26562.

[gopherbot]

Change https://golang.org/cl/127715 mentions this issue: cmd/go: revert "cmd/go: no longer eval symlinks in inDir"

[gopherbot]

Change https://golang.org/cl/127818 mentions this issue: cmd/go: cache results of EvalSymlinks

[rsc]

I'd still really like to find a way to make the approach of CL 127635 - stop evaluating symlinks at all - work. I'll try to take a look in the next couple weeks.

[bcmills]

I'd still really like to find a way to make the approach of CL 127635 - stop evaluating symlinks at all - work.

If anyone wants to make another run at this, the description for CL 127715 includes the output of the failing test.

[gopherbot]

Change https://go.dev/cl/511915 mentions this issue: cmd/go: cache results of EvalSymlinks

[bcmills]

I'm able to consistently reproduce this while trying to test a fix for #64702.

[gopherbot]

Change https://go.dev/cl/563595 mentions this issue: cmd/go: change computeTestInputsID to use str.HasFilePathPrefix