From a9bd9c230fb066674ad51dd75db5cbde16a271f2 Mon Sep 17 00:00:00 2001 From: Rohan Challa Date: Thu, 6 Feb 2020 13:59:27 -0500 Subject: [PATCH] internal/lsp/cache: improve ModTidyHandle cache key This change alters the key that is used to cache go.mod diagnostic changes, in particular it replaces using the snapshot ID with a string of all the imports used in the module and the hashed contents of the go.mod file. This reduces the number of times that we run "go mod tidy" to only when we detect import changes or the go.mod file is changed. Change-Id: Icf49db34f44a4ae4772fff6dfb8b9a6955a8e2d6 Reviewed-on: https://go-review.googlesource.com/c/tools/+/218238 Reviewed-by: Rebecca Stambler Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot --- internal/lsp/cache/check.go | 23 +++++++++++++++++++++++ internal/lsp/cache/mod_tidy.go | 29 +++++++++++++++++++++-------- internal/lsp/mod/diagnostics.go | 18 +++++++++++++++--- internal/lsp/source/view.go | 2 +- 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index b119f24b644..9b3c0fdb131 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -13,6 +13,7 @@ import ( "go/types" "path" "sort" + "strings" "sync" "golang.org/x/tools/go/packages" @@ -209,6 +210,28 @@ func (ph *packageHandle) MissingDependencies() []string { return md } +func hashImports(ctx context.Context, wsPackages []source.PackageHandle) (string, error) { + results := make(map[string]bool) + var imports []string + for _, ph := range wsPackages { + // Check package since we do not always invalidate the metadata. + pkg, err := ph.Check(ctx) + if err != nil { + return "", err + } + for _, path := range pkg.Imports() { + imp := path.PkgPath() + if _, ok := results[imp]; !ok { + results[imp] = true + imports = append(imports, imp) + } + } + } + sort.Strings(imports) + hashed := strings.Join(imports, ",") + return hashContents([]byte(hashed)), nil +} + func (ph *packageHandle) Cached() (source.Package, error) { return ph.cached() } diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go index dbf4ecc235d..d6bb7c9d68c 100644 --- a/internal/lsp/cache/mod_tidy.go +++ b/internal/lsp/cache/mod_tidy.go @@ -29,9 +29,10 @@ const ModTidyError = "go mod tidy" const SyntaxError = "syntax" type parseModKey struct { - view string - snapshotID uint64 - cfg string + view string + imports string + gomod string + cfg string } type modTidyHandle struct { @@ -90,16 +91,28 @@ func (mth *modTidyHandle) Tidy(ctx context.Context) (*modfile.File, *protocol.Co return data.origParsedFile, data.origMapper, data.missingDeps, data.parseErrors, data.err } -func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle) source.ModTidyHandle { +func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle) (source.ModTidyHandle, error) { realURI, tempURI := s.view.ModFiles() cfg := s.View().Config(ctx) options := s.View().Options() folder := s.View().Folder().Filename() + wsPackages, err := s.WorkspacePackages(ctx) + if ctx.Err() != nil { + return nil, ctx.Err() + } + if err != nil { + return nil, err + } + imports, err := hashImports(ctx, wsPackages) + if err != nil { + return nil, err + } key := parseModKey{ - view: folder, - snapshotID: s.ID(), - cfg: hashConfig(cfg), + view: folder, + imports: imports, + gomod: realfh.Identity().Identifier, + cfg: hashConfig(cfg), } h := s.view.session.cache.store.Bind(key, func(ctx context.Context) interface{} { data := &modTidyData{} @@ -194,7 +207,7 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle) handle: h, file: realfh, cfg: cfg, - } + }, nil } // extractModParseErrors processes the raw errors returned by modfile.Parse, diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index b984df9b1da..647bab3b0c0 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go @@ -35,7 +35,11 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File if err != nil { return nil, nil, err } - _, _, missingDeps, parseErrors, err := snapshot.ModTidyHandle(ctx, realfh).Tidy(ctx) + mth, err := snapshot.ModTidyHandle(ctx, realfh) + if err != nil { + return nil, nil, err + } + _, _, missingDeps, parseErrors, err := mth.Tidy(ctx) if err != nil { return nil, nil, err } @@ -61,7 +65,11 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File } func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, realfh source.FileHandle, diags []protocol.Diagnostic) []protocol.CodeAction { - _, _, _, parseErrors, err := snapshot.ModTidyHandle(ctx, realfh).Tidy(ctx) + mth, err := snapshot.ModTidyHandle(ctx, realfh) + if err != nil { + return nil + } + _, _, _, parseErrors, err := mth.Tidy(ctx) if err != nil { return nil } @@ -126,7 +134,11 @@ func SuggestedGoFixes(ctx context.Context, snapshot source.Snapshot, gofh source if err != nil { return nil, err } - realFile, realMapper, missingDeps, _, err := snapshot.ModTidyHandle(ctx, realfh).Tidy(ctx) + mth, err := snapshot.ModTidyHandle(ctx, realfh) + if err != nil { + return nil, err + } + realFile, realMapper, missingDeps, _, err := mth.Tidy(ctx) if err != nil { return nil, err } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index a04fb644cba..551ff0d96e5 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -39,7 +39,7 @@ type Snapshot interface { // ModTidyHandle returns a ModTidyHandle for the given go.mod file handle. // This function can have no data or error if there is no modfile detected. - ModTidyHandle(ctx context.Context, fh FileHandle) ModTidyHandle + ModTidyHandle(ctx context.Context, fh FileHandle) (ModTidyHandle, error) // PackageHandles returns the PackageHandles for the packages that this file // belongs to.