Skip to content

Commit

Permalink
internal/lsp: in degraded mode, limit the workspace to active packages
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
findleyr committed Jul 26, 2021
1 parent f093871 commit 07bc1bf
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 25 deletions.
12 changes: 2 additions & 10 deletions internal/lsp/cache/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,16 +207,8 @@ func (s *snapshot) workspaceParseMode(id packageID) source.ParseMode {
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
}
Expand Down
10 changes: 6 additions & 4 deletions internal/lsp/cache/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,15 @@ func (s *Session) SetProgressTracker(tracker *progress.Tracker) {
}

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))
}

Expand Down
69 changes: 64 additions & 5 deletions internal/lsp/cache/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,50 @@ func (s *snapshot) workspacePackageIDs() (ids []packageID) {
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()
Expand Down Expand Up @@ -870,8 +914,23 @@ func (s *snapshot) knownFilesInDir(ctx context.Context, dir span.URI) []span.URI
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
}
Expand All @@ -886,12 +945,12 @@ func (s *snapshot) WorkspacePackages(ctx context.Context) ([]source.Package, err
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
Expand Down Expand Up @@ -1263,7 +1322,7 @@ func (s *snapshot) GetCriticalError(ctx context.Context) *source.CriticalError {
// 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),
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn
}

// 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
}
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/mod/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func DiagnosticsForMod(ctx context.Context, snapshot source.Snapshot, fh source.
}

// 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)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/source/completion/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func (c *completer) packageNameCompletions(ctx context.Context, fileURI span.URI
// 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
}
Expand Down
7 changes: 5 additions & 2 deletions internal/lsp/source/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,11 @@ type Snapshot interface {
// 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
Expand Down
4 changes: 3 additions & 1 deletion internal/lsp/source/workspace_symbol.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,9 @@ func (sc *symbolCollector) collectPackages(ctx context.Context, views []View) ([
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
}
Expand Down

0 comments on commit 07bc1bf

Please sign in to comment.