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/gopls: excessive invocations of "go mod tidy" #38816

Closed
pwaller opened this issue May 2, 2020 · 5 comments
Closed

x/tools/gopls: excessive invocations of "go mod tidy" #38816

pwaller opened this issue May 2, 2020 · 5 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@pwaller
Copy link
Contributor

pwaller commented May 2, 2020

Running gopls from golang/tools@542909f (latest as of writing).

Go 1.14.2; Ubuntu 20.04, amd64.

What did you do?

While working on a Go project whose working directory happened to contain many non-go files (millions of xml files), I discovered that my machine was becoming bogged down by something. In the end I discovered tens of "go mod tidy" processes all competing with each other to stat all of these files. (That cmd/go bug is covered by #38791).

What did you expect to see?

Normal operation of my computer (not progressively slowing down with ever more procesess).

What did you see instead?

CPU spiral of doom, with >30 go mod tidy processes running and the computer becoming progressively more unusable.

Discussion

This highlighted that go mod tidy is being run a lot, and it's not always a cheap command. As I understand it, this command will have to read the directory entries of all directories below the Go module it's run on in order to find Go files, and that alone is still quite expensive in my setup, even if #38791 is addressed.

@mvdan and @bcmills discussed that gopls may be able in principle to do better at minimizing go mod tidy invocations. I think this should be the primary thing which is fixed, since I care about battery life of my laptop.

There is another concern here, which is that gopls allowed so many concurrent invocations of go mod tidy which were stuck competing for the same resources. It seems that these should be gated, and some sort of timeout/warning in place if it takes excessively longer than expected.

/cc @stamblerre @ianthehat

@gopherbot gopherbot added this to the Unreleased milestone May 2, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels May 2, 2020
@bcmills
Copy link
Contributor

bcmills commented May 4, 2020

There is another concern here, which is that gopls allowed so many concurrent invocations of go mod tidy which were stuck competing for the same resources.

Since go mod tidy operates on the whole package graph, I would be surprised if there is any benefit to running it more than once at a time.

Consider guarding it with a semaphore to limit to one invocation at a time, and/or signaling the previous go mod tidy with SIGTERM (and waiting for it to exit) before starting a new run.

@stamblerre
Copy link
Contributor

Thanks for following up on this. I agree that we shouldn't be running go mod tidy concurrently. We have logic in place to handle cancellation, so it sounds like a bug in that code.

@pwaller, if you have a public repro case, that would be helpful, but no worries if not - this sounds simple enough to reproduce.

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 May 5, 2020
@pwaller
Copy link
Contributor Author

pwaller commented May 9, 2020

No public repro case to hand, sorry. I believe it should be straightforward though. I have around a million files below my directory, FWIW. I don't think it should require this many before go mod tidy becomes noticably slow.

@stamblerre
Copy link
Contributor

stamblerre commented Jun 25, 2020

@pwaller: Are you able to reproduce this with the latest gopls pre-release (GO111MODULE=on go get golang.org/x/tools/gopls@0.4.2-pre1)? We've made a number of changes to the way that we run the go command, so I'm hoping that fixes the cancellation issues.

@stamblerre stamblerre added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 9, 2020
@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v1.0.0 Jul 22, 2020
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v.0.4.5 Aug 9, 2020
@golang golang locked and limited conversation to collaborators Aug 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants