From e435455aa18e448e5d7d5637d7d33e2c42d0468e Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Mon, 12 Apr 2021 18:01:18 -0400 Subject: [PATCH] internal/lsp: introduce MemoryMode We still hear from users for whom gopls uses too much memory. My efforts to reduce memory usage while maintaining functionality are proving fruitless, so perhaps it's time to accept some functionality loss. DegradeClosed MemoryMode typechecks all packages in ParseExported mode unless they have a file open. This should dramatically reduce memory usage in monorepo-style scenarious, where a ton of packages are in the workspace and the user might plausibly want to edit any of them. (Otherwise they should consider using directory filters.) The cost is that features that work across multiple packages...won't. Find references, for example, will only find uses in open packages or in the exported declarations of closed packages. The current implementation is a bit leaky; we keep the ParseFull packages in memory even once all their files are closed. This is related to a general failure on our part to drop unused packages from the snapshot, so I'm not going to try to fix it here. Updates golang/go#45457, golang/go#45363. Change-Id: I38b2aeeff81a1118024aed16a3b75e18f17893e2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/310170 Trust: Heschi Kreinick Run-TryBot: Heschi Kreinick gopls-CI: kokoro Reviewed-by: Robert Findley TryBot-Result: Go Bot --- gopls/doc/settings.md | 18 ++++++++++++++++++ internal/lsp/cache/check.go | 26 +++++++++++++++++++++----- internal/lsp/cache/snapshot.go | 6 +++--- internal/lsp/cache/view.go | 3 +++ internal/lsp/source/api_json.go | 22 ++++++++++++++++++++++ internal/lsp/source/options.go | 24 ++++++++++++++++++++++++ 6 files changed, 91 insertions(+), 8 deletions(-) diff --git a/gopls/doc/settings.md b/gopls/doc/settings.md index 1c1b1e68b4..963ef002dd 100644 --- a/gopls/doc/settings.md +++ b/gopls/doc/settings.md @@ -72,6 +72,24 @@ Include only project_a, but not node_modules inside it: `-`, `+project_a`, `-pro Default: `[]`. +#### **memoryMode** *enum* + +**This setting is experimental and may be deleted.** + +memoryMode controls the tradeoff `gopls` makes between memory usage and +correctness. + +Values other than `Normal` are untested and may break in surprising ways. + +Must be one of: + +* `"DegradeClosed"`: In DegradeClosed mode, `gopls` will collect less information about +packages without open files. As a result, features like Find +References and Rename will miss results in such packages. + +* `"Normal"` +Default: `"Normal"`. + #### **expandWorkspaceToModule** *bool* **This setting is experimental and may be deleted.** diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 65c33717ee..50f75c083d 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -187,11 +187,27 @@ func (s *snapshot) buildKey(ctx context.Context, id packageID, mode source.Parse } func (s *snapshot) workspaceParseMode(id packageID) source.ParseMode { - if _, ws := s.isWorkspacePackage(id); ws { - return source.ParseFull - } else { + s.mu.Lock() + defer s.mu.Unlock() + _, ws := s.workspacePackages[id] + if !ws { return source.ParseExported } + 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 + } + } + return source.ParseExported } func checkPackageKey(id packageID, pghs []*parseGoHandle, cfg *packages.Config, deps []packageHandleKey, mode source.ParseMode, experimentalKey bool) packageHandleKey { @@ -547,7 +563,7 @@ func (s *snapshot) depsErrors(ctx context.Context, pkg *pkg) ([]*source.Diagnost } directImporter := depsError.ImportStack[directImporterIdx] - if _, ok := s.isWorkspacePackage(packageID(directImporter)); ok { + if s.isWorkspacePackage(packageID(directImporter)) { continue } relevantErrors = append(relevantErrors, depsError) @@ -582,7 +598,7 @@ func (s *snapshot) depsErrors(ctx context.Context, pkg *pkg) ([]*source.Diagnost for _, depErr := range relevantErrors { for i := len(depErr.ImportStack) - 1; i >= 0; i-- { item := depErr.ImportStack[i] - if _, ok := s.isWorkspacePackage(packageID(item)); ok { + if s.isWorkspacePackage(packageID(item)) { break } diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index b7f0bf60ae..d2fa8477b9 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -963,12 +963,12 @@ func isCommandLineArguments(s string) bool { return strings.Contains(s, "command-line-arguments") } -func (s *snapshot) isWorkspacePackage(id packageID) (packagePath, bool) { +func (s *snapshot) isWorkspacePackage(id packageID) bool { s.mu.Lock() defer s.mu.Unlock() - scope, ok := s.workspacePackages[id] - return scope, ok + _, ok := s.workspacePackages[id] + return ok } func (s *snapshot) FindFile(uri span.URI) source.VersionedFileHandle { diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index af00c3991c..a3d6941345 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -254,6 +254,9 @@ func minorOptionsChange(a, b *source.Options) bool { if !reflect.DeepEqual(a.DirectoryFilters, b.DirectoryFilters) { return false } + if a.MemoryMode != b.MemoryMode { + return false + } aBuildFlags := make([]string, len(a.BuildFlags)) bBuildFlags := make([]string, len(b.BuildFlags)) copy(aBuildFlags, a.BuildFlags) diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index 43ce2ca348..6d56c38a88 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -44,6 +44,28 @@ var GeneratedAPIJSON = &APIJSON{ Status: "", Hierarchy: "build", }, + { + Name: "memoryMode", + Type: "enum", + Doc: "memoryMode controls the tradeoff `gopls` makes between memory usage and\ncorrectness.\n\nValues other than `Normal` are untested and may break in surprising ways.\n", + EnumKeys: EnumKeys{ + ValueType: "", + Keys: nil, + }, + EnumValues: []EnumValue{ + { + Value: "\"DegradeClosed\"", + Doc: "`\"DegradeClosed\"`: In DegradeClosed mode, `gopls` will collect less information about\npackages without open files. As a result, features like Find\nReferences and Rename will miss results in such packages.\n", + }, + { + Value: "\"Normal\"", + Doc: "", + }, + }, + Default: "\"Normal\"", + Status: "experimental", + Hierarchy: "build", + }, { Name: "expandWorkspaceToModule", Type: "bool", diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 5a5d218851..c88f95b0a5 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -107,6 +107,7 @@ func DefaultOptions() *Options { BuildOptions: BuildOptions{ ExpandWorkspaceToModule: true, ExperimentalPackageCacheKey: true, + MemoryMode: ModeNormal, }, UIOptions: UIOptions{ DiagnosticOptions: DiagnosticOptions{ @@ -221,6 +222,12 @@ type BuildOptions struct { // Include only project_a, but not node_modules inside it: `-`, `+project_a`, `-project_a/node_modules` DirectoryFilters []string + // MemoryMode controls the tradeoff `gopls` makes between memory usage and + // correctness. + // + // Values other than `Normal` are untested and may break in surprising ways. + MemoryMode MemoryMode `status:"experimental"` + // ExpandWorkspaceToModule instructs `gopls` to adjust the scope of the // workspace to find the best available module root. `gopls` first looks for // a go.mod file in any parent directory of the workspace folder, expanding @@ -550,6 +557,16 @@ const ( Structured HoverKind = "Structured" ) +type MemoryMode string + +const ( + ModeNormal MemoryMode = "Normal" + // In DegradeClosed mode, `gopls` will collect less information about + // packages without open files. As a result, features like Find + // References and Rename will miss results in such packages. + ModeDegradeClosed MemoryMode = "DegradeClosed" +) + type OptionResults []OptionResult type OptionResult struct { @@ -753,6 +770,13 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{}) filters = append(filters, strings.TrimRight(filepath.FromSlash(filter), "/")) } o.DirectoryFilters = filters + case "memoryMode": + if s, ok := result.asOneOf( + string(ModeNormal), + string(ModeDegradeClosed), + ); ok { + o.MemoryMode = MemoryMode(s) + } case "completionDocumentation": result.setBool(&o.CompletionDocumentation) case "usePlaceholders":