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.

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

x/tools/gopls: vet the DegradeClosed memory mode and move out of experimental #46902

Closed
findleyr opened this issue Jun 24, 2021 · 7 comments
Closed

x/tools/gopls: vet the DegradeClosed memory mode and move out of experimental #46902

findleyr opened this issue Jun 24, 2021 · 7 comments
Labels
FrozenDueToAge gopls/performance Issues related to gopls performance (CPU, memory, etc). 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.15.0

Comments

@findleyr
Copy link
Member

https://golang.org/cl/330529 fixed loading of std and cmd, resulting in better features for std and cmd but worse resource usage.

"memoryMode": "DegradedClosed" should per my recollection revert us to the old model, but it seems not to work. We should fix this. More generally, I think the degraded memory model could be useful for many users, and we should vet it and move it out of experimental.

CC @stamblerre
@findleyr findleyr added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 24, 2021
@findleyr findleyr added this to the gopls/v0.7.1 milestone Jun 24, 2021
@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 Jun 24, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/333689 mentions this issue: internal/lsp: improve package search in a couple places

gopherbot pushed a commit to golang/tools that referenced this issue Jul 13, 2021
@findleyr
internal/lsp: improve package search in a couple places
d36a54b 
When we open a file in a package, independent of whether it is in the
workspace, we type check in ParseFull mode. However, several other
code paths don't find this better parse mode.

We need a better abstraction, but for now improve a couple code paths
specifically for the purpose of fixing Hover content.

Updates golang/go#46158
Updates golang/go#46902

Change-Id: I34c0432fdba406d569ea963ab4366336068767a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333689
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/336410 mentions this issue: internal/lsp: in degraded mode, limit the workspace to active packages

@stamblerre stamblerre modified the milestones: gopls/v0.7.1, gopls/v0.7.2 Jul 26, 2021
gopherbot pushed a commit to golang/tools that referenced this issue Jul 26, 2021
@findleyr
internal/lsp: in degraded mode, limit the workspace to active packages
07bc1bf 
In my testing, the gopls degraded memory mode (currently set via
"memoryMode": "DegradeClosed") did not save as much memory as expected
due to still type checking all packages in the workspace (even if in
ParseExported mode). It is also annoying to get incomplete results from
references and renaming.

I think we can (and should) fix both problems: don't even consider
packages that aren't 'reachable' via open files, but fully type check
the reverse transitive closure of the packages you're working on. This
CL does exactly that, by swapping out the concept of 'workspace
packages' with 'active packages'.

In testing, this decreased my memory footprint while working on std by
3-4x when compared to normal mode, and 2x when compared to the previous
implementation of DegradeClosed.

It still needs more testing before we move this option out of
experimental.

For golang/go#46902

Change-Id: I1e319d0b1607d344d27e797ce32de057d7a583f9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/336410
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@stamblerre stamblerre modified the milestones: gopls/v0.7.2, gopls/on-deck Sep 9, 2021
@findleyr findleyr added the gopls/performance Issues related to gopls performance (CPU, memory, etc). label May 10, 2022
@findleyr findleyr changed the title x/tools/gopls: vet the DegradedClosed memory mode and move out of experimental x/tools/gopls: vet the DegradeClosed memory mode and move out of experimental Feb 28, 2023
@findleyr findleyr modified the milestones: gopls/later, gopls/v0.12.0 Feb 28, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/472179 mentions this issue: gopls/internal/lsp/source: remove the "memoryMode" setting

@findleyr
Copy link
Member Author

I'm not sure we should remove the 'memoryMode' setting yet, until we have more data on the performance of gopls' new architecture. Moving this to v0.13.0.

@findleyr findleyr modified the milestones: gopls/v0.12.0, gopls/v0.13.0 Mar 28, 2023
@adonovan
Copy link
Member

Perhaps we can provide users all the control the need with just two parameters to delay the onset of type checking and of go.analysis?

@findleyr findleyr modified the milestones: gopls/v0.14.0, gopls/v0.15.0 Sep 27, 2023
@adonovan
Copy link
Member

I'm not sure we should remove the 'memoryMode' setting yet, until we have more data on the performance of gopls' new architecture. Moving this to v0.13.0.

Should we remove it for v0.15?

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/548739 mentions this issue: gopls/internal/settings: remove MemoryMode option

@golang golang locked and limited conversation to collaborators Dec 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls/performance Issues related to gopls performance (CPU, memory, etc). 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.15.0
Development

No branches or pull requests
4 participants
@adonovan @stamblerre @gopherbot @findleyr