internal/lsp: in degraded mode, limit the workspace to active packages
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>
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 89094b0..287451f 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -207,16 +207,8 @@
if s.view.Options().MemoryMode == source.ModeNormal {
return source.ParseFull
}
-
- // Degraded mode. Check for open files.
- m, ok := s.metadata[id]
- if !ok {
- return source.ParseExported
- }
- for _, cgf := range m.compiledGoFiles {
- if s.isOpenLocked(cgf) {
- return source.ParseFull
- }
+ if s.isActiveLocked(id, nil) {
+ return source.ParseFull
}
return source.ParseExported
}
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 2cd85b9..7e78811 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -140,13 +140,15 @@
}
func (s *Session) Shutdown(ctx context.Context) {
+ var views []*View
s.viewMu.Lock()
- defer s.viewMu.Unlock()
- for _, view := range s.views {
- view.shutdown(ctx)
- }
+ views = append(views, s.views...)
s.views = nil
s.viewMap = nil
+ s.viewMu.Unlock()
+ for _, view := range views {
+ view.shutdown(ctx)
+ }
event.Log(ctx, "Shutdown session", KeyShutdownSession.Of(s))
}
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index c741885..a763f97 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -725,6 +725,50 @@
return ids
}
+func (s *snapshot) activePackageIDs() (ids []packageID) {
+ if s.view.Options().MemoryMode == source.ModeNormal {
+ return s.workspacePackageIDs()
+ }
+
+ s.mu.Lock()
+ defer s.mu.Unlock()
+
+ seen := make(map[packageID]bool)
+ for id := range s.workspacePackages {
+ if s.isActiveLocked(id, seen) {
+ ids = append(ids, id)
+ }
+ }
+ return ids
+}
+
+func (s *snapshot) isActiveLocked(id packageID, seen map[packageID]bool) (active bool) {
+ if seen == nil {
+ seen = make(map[packageID]bool)
+ }
+ if seen, ok := seen[id]; ok {
+ return seen
+ }
+ defer func() {
+ seen[id] = active
+ }()
+ m, ok := s.metadata[id]
+ if !ok {
+ return false
+ }
+ for _, cgf := range m.compiledGoFiles {
+ if s.isOpenLocked(cgf) {
+ return true
+ }
+ }
+ for _, dep := range m.deps {
+ if s.isActiveLocked(dep, seen) {
+ return true
+ }
+ }
+ return false
+}
+
func (s *snapshot) getWorkspacePkgPath(id packageID) packagePath {
s.mu.Lock()
defer s.mu.Unlock()
@@ -870,8 +914,23 @@
return files
}
-func (s *snapshot) WorkspacePackages(ctx context.Context) ([]source.Package, error) {
- phs, err := s.workspacePackageHandles(ctx)
+func (s *snapshot) workspacePackageHandles(ctx context.Context) ([]*packageHandle, error) {
+ if err := s.awaitLoaded(ctx); err != nil {
+ return nil, err
+ }
+ var phs []*packageHandle
+ for _, pkgID := range s.workspacePackageIDs() {
+ ph, err := s.buildPackageHandle(ctx, pkgID, s.workspaceParseMode(pkgID))
+ if err != nil {
+ return nil, err
+ }
+ phs = append(phs, ph)
+ }
+ return phs, nil
+}
+
+func (s *snapshot) ActivePackages(ctx context.Context) ([]source.Package, error) {
+ phs, err := s.activePackageHandles(ctx)
if err != nil {
return nil, err
}
@@ -886,12 +945,12 @@
return pkgs, nil
}
-func (s *snapshot) workspacePackageHandles(ctx context.Context) ([]*packageHandle, error) {
+func (s *snapshot) activePackageHandles(ctx context.Context) ([]*packageHandle, error) {
if err := s.awaitLoaded(ctx); err != nil {
return nil, err
}
var phs []*packageHandle
- for _, pkgID := range s.workspacePackageIDs() {
+ for _, pkgID := range s.activePackageIDs() {
ph, err := s.buildPackageHandle(ctx, pkgID, s.workspaceParseMode(pkgID))
if err != nil {
return nil, err
@@ -1263,7 +1322,7 @@
// Even if packages didn't fail to load, we still may want to show
// additional warnings.
if loadErr == nil {
- wsPkgs, _ := s.WorkspacePackages(ctx)
+ wsPkgs, _ := s.ActivePackages(ctx)
if msg := shouldShowAdHocPackagesWarning(s, wsPkgs); msg != "" {
return &source.CriticalError{
MainError: errors.New(msg),
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index ef23372..d931f51 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -211,7 +211,7 @@
}
// Diagnose all of the packages in the workspace.
- wsPkgs, err := snapshot.WorkspacePackages(ctx)
+ wsPkgs, err := snapshot.ActivePackages(ctx)
if s.shouldIgnoreError(ctx, snapshot, err) {
return
}
diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go
index 625bc63..4b4d0cb 100644
--- a/internal/lsp/mod/diagnostics.go
+++ b/internal/lsp/mod/diagnostics.go
@@ -86,7 +86,7 @@
}
// Packages in the workspace can contribute diagnostics to go.mod files.
- wspkgs, err := snapshot.WorkspacePackages(ctx)
+ wspkgs, err := snapshot.ActivePackages(ctx)
if err != nil && !source.IsNonFatalGoModError(err) {
event.Error(ctx, fmt.Sprintf("workspace packages: diagnosing %s", pm.URI), err)
}
diff --git a/internal/lsp/source/completion/package.go b/internal/lsp/source/completion/package.go
index d927fef..0ed66e6 100644
--- a/internal/lsp/source/completion/package.go
+++ b/internal/lsp/source/completion/package.go
@@ -213,7 +213,7 @@
// file. This also includes test packages for these packages (<pkg>_test) and
// the directory name itself.
func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI span.URI, prefix string) (packages []candidate, err error) {
- workspacePackages, err := snapshot.WorkspacePackages(ctx)
+ workspacePackages, err := snapshot.ActivePackages(ctx)
if err != nil {
return nil, err
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 74b77ca..9079ca5 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -156,8 +156,11 @@
// in TypecheckWorkspace mode.
KnownPackages(ctx context.Context) ([]Package, error)
- // WorkspacePackages returns the snapshot's top-level packages.
- WorkspacePackages(ctx context.Context) ([]Package, error)
+ // ActivePackages returns the packages considered 'active' in the workspace.
+ //
+ // In normal memory mode, this is all workspace packages. In degraded memory
+ // mode, this is just the reverse transitive closure of open packages.
+ ActivePackages(ctx context.Context) ([]Package, error)
// GetCriticalError returns any critical errors in the workspace.
GetCriticalError(ctx context.Context) *CriticalError
diff --git a/internal/lsp/source/workspace_symbol.go b/internal/lsp/source/workspace_symbol.go
index 18583ae..e9e3f0d 100644
--- a/internal/lsp/source/workspace_symbol.go
+++ b/internal/lsp/source/workspace_symbol.go
@@ -311,7 +311,9 @@
if err != nil {
return nil, err
}
- workspacePackages, err := snapshot.WorkspacePackages(ctx)
+ // TODO(rfindley): this can result in incomplete information in degraded
+ // memory mode.
+ workspacePackages, err := snapshot.ActivePackages(ctx)
if err != nil {
return nil, err
}
