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: bug - loading packages startup penalty occurs for each session with remote gopls server. #51040

Open
mattysweeps opened this issue Feb 6, 2022 · 3 comments
Labels
gopls/performance gopls NeedsInvestigation Tools
Milestone

Comments

@mattysweeps
Copy link

@mattysweeps mattysweeps commented Feb 6, 2022

gopls version

$ gopls version
golang.org/x/tools/gopls v0.7.5
    golang.org/x/tools/gopls@v0.7.5 h1:8Az52YwcFXTWPvrRomns1C0N+zlgTyyPKWvRazO9GG8=

My goal is to reduce the startup penalty when opening my editor (vim) for a large golang project.
An example is kubernetes. It takes 80 seconds for the "Loading packages" stage to finish. I like to open and close my editor as needed. This means each time I open my editor I might have to wait a considerable amount of time before I can explore the code.

I tried to run gopls in the background using gopls -listen 127.0.0.1:8118 and configuring my editor (nvim w/ coc) to execute gopls -remote 127.0.0.1:8118. LSP functionality works, but each time I open the editor I have the same loading penalty. This means there's no benefit to running gopls in the background.

https://pastebin.com/yfucLgc9
My coc-ssettings.json config.

Log message I see:
https://github.com/golang/tools/blob/master/internal/lsp/general.go#L233

It might seem that the gopls -remote command is the problem, but I've done some testing to show that's not the case. I created a small test program which reads/writes back and forth from stdin/stdout to a TCP connection with the gopls -address 127.0.0.1:8118 server.

https://go.dev/play/p/WM2R5Q5borF

I compiled this program and configured my editor to invoke it when loading the LSP for go code. Again, the LSP functionality works, so the connection is good. However, the loading packages penalty persists, meaning this is in fact happening server-side for each session I make.

@gopherbot gopherbot added Tools gopls labels Feb 6, 2022
@gopherbot gopherbot added this to the Unreleased milestone Feb 6, 2022
@findleyr
Copy link
Contributor

@findleyr findleyr commented Feb 7, 2022

@mattysweeps are you closing and then opening-up vim? If so, the type-checked packages will be aggressively dropped from cache, and you'll pay the startup cost anew. This is a behavior we could change, but that's how it works now.

If you're opening multiple vim sessions simultaneously, I think we should get cache hits for a lot of that startup cost, so there might be a bug. We can't cache go list, so you'll always pay that price, but that should only be a fraction (say ~1/4th) of startup.

@findleyr findleyr added the WaitingForInfo label Feb 7, 2022
@mattysweeps
Copy link
Author

@mattysweeps mattysweeps commented Feb 7, 2022

@mattysweeps are you closing and then opening-up vim? If so, the type-checked packages will be aggressively dropped from cache, and you'll pay the startup cost anew. This is a behavior we could change, but that's how it works now.

If you're opening multiple vim sessions simultaneously, I think we should get cache hits for a lot of that startup cost, so there might be a bug. We can't cache go list, so you'll always pay that price, but that should only be a fraction (say ~1/4th) of startup.

The former: I'm opening and closing vim. One thing to add: it doesn't seem like there's any difference in penalty based on when I open the window. The penalty is the same regardless of the remote gopls was just opened, or opened minutes ago.

I just tested opening two vim sessions in parallel using the same remote gopls. The results are the same: the second window takes an equal amount of time to load packages

@findleyr
Copy link
Contributor

@findleyr findleyr commented Feb 7, 2022

I just tested opening two vim sessions in parallel using the same remote gopls. The results are the same: the second window takes an equal amount of time to load packages

Ack, thanks for testing.

@findleyr findleyr removed this from the Unreleased milestone Feb 7, 2022
@findleyr findleyr added this to the gopls/v0.8.0 milestone Feb 7, 2022
@findleyr findleyr added NeedsInvestigation and removed WaitingForInfo labels Feb 7, 2022
@suzmue suzmue removed this from the gopls/v0.8.0 milestone Feb 11, 2022
@suzmue suzmue added this to the gopls/on-deck milestone Feb 11, 2022
@findleyr findleyr added the gopls/performance label Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/performance gopls NeedsInvestigation Tools
Projects
None yet
Development

No branches or pull requests

4 participants