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: re-initialize workspace when the go binary changes #49883

Closed
mitar opened this issue Nov 30, 2021 · 8 comments
Closed

x/tools/gopls: re-initialize workspace when the go binary changes #49883

mitar opened this issue Nov 30, 2021 · 8 comments
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.

Comments

@mitar
Copy link
Contributor

mitar commented Nov 30, 2021

What did you do?

I have been developing a library using Go 1.16 and everything was working fine. Then I decided to upgrade Go on my system to 1.17.3 and use it to continue developing the library. But gopls started consuming a lot of memory, to the amount that it gets killed by OOM. Then Vscode restarts it and everything starts again.

Build info

golang.org/x/tools/gopls v0.7.3
    golang.org/x/tools/gopls@v0.7.3 h1:Lru57ht8vtDMouRskFC085VAjBAZRAISd/lwvwOOV0Q=
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/google/go-cmp@v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
    golang.org/x/sync@v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
    golang.org/x/sys@v0.0.0-20210809222454-d867a43fc93e h1:WUoyKPm6nCo1BnNUvPGnFG3T5DUVem42yDJZZ4CNxMA=
    golang.org/x/text@v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M=
    golang.org/x/tools@v0.1.8-0.20211014194737-fc98fb2abd48 h1:hk7xRoeg0CG1nRLsd5BZLDUgVpA9bnKylGk1p2/BPH0=
    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
    honnef.co/go/tools@v0.2.0 h1:ws8AfbgTX3oIczLPNPCu5166oBg9ST2vNs0rcht+mDE=
    mvdan.cc/gofumpt@v0.1.1 h1:bi/1aS/5W00E2ny5q65w9SnKpWEF/UIOqDYBILpo9rA=
    mvdan.cc/xurls/v2@v2.3.0 h1:59Olnbt67UKpxF1EwVBopJvkSUBmgtb468E4GVWIZ1I=
@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 Nov 30, 2021
@gopherbot gopherbot added this to the Unreleased milestone Nov 30, 2021
@findleyr
Copy link
Contributor

Possibly related: #49035, though my expectation was that this only affected the high-water mark (that may also be the case here; it is more concerning if the steady-state memory is significantly higher).

Thanks for the repro, and the profiles.

@mitar
Copy link
Contributor Author

mitar commented Dec 1, 2021

One more thing, I have generally pretty generic VSCode settings, but I do have experimentalWorkspaceModule enabled.

@suzmue suzmue modified the milestones: Unreleased, gopls/unplanned Dec 2, 2021
@findleyr
Copy link
Contributor

findleyr commented Dec 3, 2021

Ok, this is interesting. Looking at your 19GB.withNames heap profile, things are not as I would have expected:

      flat  flat%   sum%        cum   cum%
    4.25GB 45.81% 45.81%     4.26GB 46.01%  golang.org/x/tools/internal/lsp/cache.expandErrors
    1.84GB 19.85% 65.66%     1.84GB 19.85%  strings.(*Builder).WriteString (inline)
    1.21GB 13.07% 78.73%     3.11GB 33.56%  golang.org/x/tools/internal/lsp/cache.typeErrorDiagnostics
    0.58GB  6.23% 84.97%     0.58GB  6.23%  go/types.(*Checker).recordTypeAndValue
    0.24GB  2.62% 87.58%     0.24GB  2.62%  go/types.(*Checker).recordUse
    0.11GB  1.21% 88.79%     0.11GB  1.21%  go/types.(*Scope).Insert
    0.10GB  1.05% 89.85%     0.10GB  1.05%  go/types.NewScope
    0.07GB  0.75% 90.60%     0.07GB  0.75%  fmt.Sprintf
    0.07GB  0.71% 91.31%     0.07GB  0.79%  go/types.(*Checker).recordSelection
    0.06GB  0.68% 91.99%     0.06GB  0.68%  go/types.(*Checker).recordDef

Half of the heap is allocated in expandErrors. I checked recent heap dumps of my own gopls, and don't see anything similar.

It seems likely that we either have a memory leak, or there were a tremendous amount of errors in memory at the high-water mark. I will try to reproduce with your repo, but any additional clues you might have would be useful.

@heschi might also find this interesting...

@mitar
Copy link
Contributor Author

mitar commented Dec 3, 2021

Few days after this behavior started and was persistent (like, even once gopls got killed, the new instance quickly degraded into the same behavior) it stopped. I do not know why. Maybe it is connected with the state of the editors and code in the editors at that particular moment, so after I continued working (or, tried to work, given instability) and closed tabs and stuff, it seems it is gone. So sadly I cannot provide more input into this.

Maybe it is connected with the fact that I had VScode running first with old go version and then I upgraded go while VScode and all tabs were left open. Maybe this confused it?

@findleyr findleyr added the gopls/performance Issues related to gopls performance (CPU, memory, etc). label Dec 6, 2021
@findleyr
Copy link
Contributor

findleyr commented Dec 6, 2021

Thanks for the follow-up. I suppose it's theoretically possible that upgrading the Go command led to an explosion in errors, due to some sort of go list thrashing?

Unfortunately tracking this down is probably not worthwhile, but we should perhaps reinitialize gopls if ever we detect that the Go binary has changed.

@findleyr findleyr changed the title x/tools/gopls: High memory usage after upgrading to Go 1.17.3 x/tools/gopls: re-initialize workspace when the Go command changes Dec 6, 2021
@findleyr findleyr removed the gopls/performance Issues related to gopls performance (CPU, memory, etc). label Dec 6, 2021
@mitar
Copy link
Contributor Author

mitar commented Dec 6, 2021

Sounds good.

@findleyr findleyr changed the title x/tools/gopls: re-initialize workspace when the Go command changes x/tools/gopls: re-initialize workspace when the go binary changes Dec 6, 2021
@hyangah
Copy link
Contributor

hyangah commented Dec 13, 2021

Is reinitializing gopls sufficient, or shouldn't it be cleaner and safer for the user or the editor (vscode) to rebuild the gopls and restart the session with the updated Go?

@findleyr
Copy link
Contributor

This is unfortunately a relatively low priority, so moving it to unplanned while we focus on 1.18 support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants