Skip to content

x/tools/gopls: expensive go list run on startup #35818

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.

Sign up for GitHub

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

Jump to bottom
Closed
myitcv opened this issue Nov 25, 2019 · 11 comments
Closed

x/tools/gopls: expensive go list run on startup #35818

myitcv opened this issue Nov 25, 2019 · 11 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.3.0

Comments

@myitcv
Copy link
Member

myitcv commented Nov 25, 2019 •
edited
Loading

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

$ go version
go version devel +6f7b96f6cb Sat Nov 23 11:00:41 2019 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20191113223546-95cb2a1a7eae => github.com/myitcvforks/tools v0.0.0-20191119111301-0222b4b716c6
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.1.8-0.20191115202509-3a792d9c32b2 => github.com/myitcvforks/tools/gopls v0.0.0-20191119111301-0222b4b716c6

Note that github.com/myitcvforks/tools v0.0.0-20191119111301-0222b4b716c6 and github.com/myitcvforks/tools/gopls v0.0.0-20191119111301-0222b4b716c6 correspond to the x/tools 95cb2a1 with 80313e1 cherry picked on top. Reason being, we can't move beyond 95cb2a1 because otherwise we start tripping over the mistmatched versions problem described in #35114

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/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-build751419054=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Started govim from the $HOME. This directory is outside of a module (GOMOD="") and not within my GOPATH.

What did you expect to see?

No activity from gopls

What did you see instead?

gopls running:

go list -e -json -compiled=true -test=true -export=false -deps=true -find=false -- ./...

I suspect (although haven't bisected) this change happened as part of https://go-review.googlesource.com/c/tools/+/204822.

The running of this query should I think at most be limited to either:

  • having a working directory within GOPATH
  • being in a module context, i.e. GOMOD != ""

Alternatively the client could be responsible for not loading gopls (or itself for that matter) unless those conditions are met, but that seems to put too much logic in the client. And also precludes the situation where the user then goes on to create a go.mod by hand, thereby creating a module context.

cc @stamblerre @matloob
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Nov 25, 2019
@gopherbot gopherbot added this to the Unreleased milestone Nov 25, 2019
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Nov 25, 2019
@myitcv myitcv mentioned this issue Nov 25, 2019
Closed
@stamblerre
Copy link
Contributor

You're correct in saying that CL 204822 changed the behavior of gopls to load all packages in the workspace at start-up rather than on demand. This allows us to do workspace-local references and rename. gopls can also handle ad-hoc packages, i.e. packages outside of GOPATH and outside of a module. They will not be handled as well as other packages, but most features should still work. What is the reason to treat these scenarios differently?

@myitcv
Copy link
Member Author

myitcv commented Dec 2, 2019

What is the reason to treat these scenarios differently?

Because otherwise you end up with situations where you run:

go list -e -json -compiled=true -test=true -export=false -deps=true -find=false -- ./...

in a directory which, in my case, is ~60GB and the command takes 10 minutes and counting.

Indeed, if you are outside of a module context or GOPATH (i.e. ad hoc) I'm not sure the concept of a workspace within which references, renames etc is defined beyond the current directory, is it?

@cespare
Copy link
Contributor

cespare commented Dec 3, 2019

I'm running into this as well (also via govim). When I start vim from some random directory (often $HOME) to do non-Go work (perhaps edit a single text file), gopls might run a very expensive go list which takes a few minutes, uses many CPU cores, and afterwards returns an error ("Error loading workspace folders") that govim exposes inside vim.

@stamblerre
Copy link
Contributor

Does govim always start gopls, even if the user is editing non-Go files? I didn't realize this when we were discussing this on the golang-tools call. I know that other extensions (citing VS Code as an example since it's the one I'm most familiar with), only start gopls if the workspace contains .go files.

@cespare
Copy link
Contributor

cespare commented Dec 3, 2019

Does govim always start gopls, even if the user is editing non-Go files?

Yes.

@stamblerre
Copy link
Contributor

I would argue that that's probably the larger issue here. I can't immediately think of an argument for why that would be necessary.

@myitcv
Copy link
Member Author

myitcv commented Dec 3, 2019

Does govim always start gopls, even if the user is editing non-Go files?

Yes.

only start gopls if the workspace contains .go files.

But underneath my $HOME directory there are .go files in sub directories, so wouldn't VSCode start gopls in this case too?

(On a related point, I seem to recall only go get is defined to work outside of GOPATH and a module context, but perhaps @jayconrod or @bcmills can clarify/confirm? Hence this go list query should probably be failing.)

My worry with this sort of logic, "start gopls when X," is that it's incumbent on LSP clients to then get this logic right. Indeed what is X?

My understanding is that we will need gopls when, i.e. X is defined as:

  1. the workspace is (within) a module context
  2. the workspace is within GOPATH
  3. you are in neither the above situations, but doing ad hoc editing of .go files in a (sub) directory of the workspace

(ignoring nested modules for one second, because that's more of a UI/UX problem to my mind)

What about when go.mod and .go files, for example, are subsequently created within the workspace?

Putting this logic in the client also means we close the door on making gopls "smart" as things change. Because if gopls is not running, the user is stuck.

Hence why I'm suggesting to sidestep the problem of clients getting this right and implement it in gopls. i.e. do away with clients needing to know about such language/tooling specific logic X and always start/connect to gopls.

@jayconrod
Copy link
Contributor

(On a related point, I seem to recall only go get is defined to work outside of GOPATH and a module context, but perhaps @jayconrod or @bcmills can clarify/confirm? Hence this go list query should probably be failing.)

In 1.14, if the go command is run in module mode outside of a module, an error will be reported if there is a need to resolve a package path to a module. For example, go list golang.org/x/net/html would fail. go get is exempt from this.

This probably won't come up much because it requires GO111MODULE explicitly set to on. By default, it's in GOPATH mode if there's no go.mod file.

@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Dec 4, 2019
@cespare cespare mentioned this issue Jan 19, 2020
Closed
@myitcv
Copy link
Member Author

myitcv commented Jan 20, 2020

I've just done a test against 0cba7a3 with partial revert of CL 214586 applied on top, in the form of CL 215239.

We are still seeing the expensive go list query run on startup, despite the workspace root being a directory outside of GOPATH and or a module context.

@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.3.0 Jan 21, 2020
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/216143 mentions this issue: internal/lsp: push more build-specific logic into the view

@myitcv
Copy link
Member Author

myitcv commented Jan 26, 2020

Thanks very much for this fix!

@myitcv myitcv mentioned this issue Mar 12, 2020
Closed
@myitcv myitcv mentioned this issue Sep 23, 2020
Closed
@golang golang locked and limited conversation to collaborators Jan 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.3.0
Development

No branches or pull requests
5 participants
@jayconrod @cespare @myitcv @stamblerre @gopherbot