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

x/tools/cmd/goimports: (unmerged CL version) slow resolution with large module cache #28518

Closed
myitcv opened this issue Oct 31, 2018 · 6 comments
Closed

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Oct 31, 2018

NOTE: this issue relates to an unmerged CL.

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

go version go1.11.1 linux/amd64

goimports as of
git fetch https://go.googlesource.com/tools refs/changes/97/142697/6 && git checkout FETCH_HEAD

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/go-modules-by-example/.gopath"
GOPROXY=""
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/go-modules-by-example/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build922624083=/tmp/go-build -gno-record-gcc-switches"

What did you do?

See repro script and output at: https://gist.github.com/myitcv/bbe8a91813e7a8fc5d2fc599013cf305

The general setup is:

  • Create a simple module that is missing an "os/exec" import (but otherwise would compile)
  • Run goimports and see that it resolves quickly
  • Fill the module cache with a load of stuff
  • Run goimports and see that it now resolves much more slowly

What did you expect to see?

Given the missing import is "os/exec", a fast goimports resolution should be possible irrespective of the state of my module cache.

What did you see instead?

A much slower goimports resolution with a large module cache: 7.5s vs 0.2s.

cc @heschik

@myitcv
Copy link
Member Author

@myitcv myitcv commented Oct 31, 2018

Per discussions with @heschik offline, we've tracked down the issue.

Now the repro (with fix applied):

https://gist.github.com/myitcv/a9fcd3c7af7ef816711c83b300ea2a9d

has the second set of goimports runs at ~0.6s.

Hence I'll close this and follow up in https://go-review.googlesource.com/c/tools/+/142697/

@myitcv myitcv closed this Oct 31, 2018
@heschik
Copy link
Contributor

@heschik heschik commented Oct 31, 2018

For posterity, the problem was that the scratch module's dependencies couldn't be resolved, but figuring that out took go list quite a while and some network access. Since it ended up failing, the result wasn't cached, making it slow every time. Setting GOPROXY=off or to $GOPATH/pkg/mod/cache prevented the network access and (for reasons unknown) allowed the build to happen.

@bcmills, is the last sentence above something you'd expect?

@myitcv
Copy link
Member Author

@myitcv myitcv commented Oct 31, 2018

the problem was that the scratch module's dependencies couldn't be resolved

Just to correct one point; the module in which we were running goimports had zero dependencies, other than stdlib.

Rather, I think the module cache was in a state that caused go list to fail/be slow/something. And it got into this state by me priming it with the result of trying to convert github.com/docker/docker to be a module.

But per https://go-review.googlesource.com/c/tools/+/142697/#message-c2037823b2f2751007f352553e57b89a555374f2, I don't think we should even have hit the path of checking the module cache.

@heschik
Copy link
Contributor

@heschik heschik commented Oct 31, 2018

Yeah, sorry, in this case the "scratch module" I was referring to is an internal implementation detail of go/packages, not your module. I'm replying to the code review comment on the code review.

@myitcv
Copy link
Member Author

@myitcv myitcv commented Oct 31, 2018

Ah I see, my bad.

@bcmills
Copy link
Member

@bcmills bcmills commented Nov 8, 2018

Setting GOPROXY=off or to $GOPATH/pkg/mod/cache prevented the network access and (for reasons unknown) allowed the build to happen.

@bcmills, is the last sentence above something you'd expect?

That is unexpected. go list should fail in exactly the cases where go build does (and succeed when go build succeeds).

@golang golang locked and limited conversation to collaborators Nov 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.