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

Jump to bottom

x/tools/gopls: memory leak in parsing #57222

Closed
findleyr opened this issue Dec 9, 2022 · 2 comments
Closed

x/tools/gopls: memory leak in parsing #57222

findleyr opened this issue Dec 9, 2022 · 2 comments
Assignees
@findleyr
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.
Milestone
gopls/v0.11.0

Comments

@findleyr
Copy link
Contributor

findleyr commented Dec 9, 2022

It looks like following recent changes to snapshot accounting, snapshot.ParseGo does not track parse keys modes correctly, causing gopls to hold onto parse results from earlier versions of a file.

This can lead to a significant memory leak over time, particularly for large files.

Fix incoming.

CC @adonovan
@findleyr findleyr added this to the gopls/v0.11.0 milestone Dec 9, 2022
@findleyr findleyr self-assigned this Dec 9, 2022
@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 Dec 9, 2022
@gopherbot
Copy link

Change https://go.dev/cl/456642 mentions this issue: gopls/internal/lsp/cache: record parse keys when they're created

@gopherbot
Copy link

Change https://go.dev/cl/456815 mentions this issue: [gopls-release-branch.0.11] gopls/internal/lsp/cache: record parse keys when they're created

gopherbot pushed a commit to golang/tools that referenced this issue Dec 12, 2022
@findleyr
[gopls-release-branch.0.11] gopls/internal/lsp/cache: record parse ke…
b82bf6f 
…ys when they're created

When we parse a file through snapshot.ParseGo, we must record the parse
key by its URI for later invalidation. This wasn't done, resulting in a
memory leak.

Fix the leak by actually recording parse keys in the parseKeysByURI map,
which was previously always empty.

Write a test that diffs the cache before and after a change, which would
have caught this bug (and others).

Fixes golang/go#57222

Change-Id: I308812bf1030276dff08c26d359433750f44849a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/456642
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
(cherry picked from commit a3eef25)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/456815
@golang golang locked and limited conversation to collaborators Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@findleyr findleyr

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.
Projects
None yet
Milestone
gopls/v0.11.0
Development

No branches or pull requests
2 participants
@gopherbot @findleyr