From 6c149bb5ef0db65038b2abbe6c6d0128f2f2cd90 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Fri, 24 Jul 2020 17:17:13 -0400 Subject: [PATCH] internal/lsp/cache: store parseGoHandles parseGoHandles have lifetimes separate from the packages they belong to. For example, a package may be invalidated by a change to one of its files, but we still want to retain the parse results for all the rest. Track them explicitly. Change-Id: I03a4ffe283bf2b252d2d838bdb2cf332cd981075 Reviewed-on: https://go-review.googlesource.com/c/tools/+/245059 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Robert Findley Reviewed-by: Rebecca Stambler --- internal/lsp/cache/check.go | 4 +-- internal/lsp/cache/parse.go | 30 ++++++++++---------- internal/lsp/cache/session.go | 1 + internal/lsp/cache/snapshot.go | 51 +++++++++++++++++++++++++++++----- 4 files changed, 61 insertions(+), 25 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 05a173fb47..abb6ba22fa 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -238,7 +238,7 @@ func (s *snapshot) parseGoHandles(ctx context.Context, files []span.URI, mode so if err != nil { return nil, err } - pghs = append(pghs, s.view.session.cache.parseGoHandle(ctx, fh, mode)) + pghs = append(pghs, s.parseGoHandle(ctx, fh, mode)) } return pghs, nil } @@ -288,7 +288,7 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source actualErrors[i] = err return } - pgh := snapshot.view.session.cache.parseGoHandle(ctx, fh, mode) + pgh := snapshot.parseGoHandle(ctx, fh, mode) pgf, fixed, err := snapshot.parseGo(ctx, pgh) if err != nil { actualErrors[i] = err diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index 8f92b37bf7..b5ce9dff6a 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -51,27 +51,31 @@ type parseGoData struct { err error // any other errors } -func (c *Cache) parseGoHandle(ctx context.Context, fh source.FileHandle, mode source.ParseMode) *parseGoHandle { +func (s *snapshot) parseGoHandle(ctx context.Context, fh source.FileHandle, mode source.ParseMode) *parseGoHandle { key := parseKey{ file: fh.FileIdentity(), mode: mode, } - parseHandle := c.store.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} { + if pgh := s.getGoFile(key); pgh != nil { + return pgh + } + parseHandle := s.view.session.cache.store.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} { snapshot := arg.(*snapshot) return parseGo(ctx, snapshot.view.session.cache.fset, fh, mode) }) - astHandle := c.store.Bind(astCacheKey(key), func(ctx context.Context, arg memoize.Arg) interface{} { + astHandle := s.view.session.cache.store.Bind(astCacheKey(key), func(ctx context.Context, arg memoize.Arg) interface{} { snapshot := arg.(*snapshot) return buildASTCache(ctx, snapshot, parseHandle) }) - return &parseGoHandle{ + pgh := &parseGoHandle{ handle: parseHandle, file: fh, mode: mode, astCacheHandle: astHandle, } + return s.addGoFile(key, pgh) } func (pgh *parseGoHandle) String() string { @@ -87,7 +91,7 @@ func (pgh *parseGoHandle) Mode() source.ParseMode { } func (s *snapshot) ParseGo(ctx context.Context, fh source.FileHandle, mode source.ParseMode) (*source.ParsedGoFile, error) { - pgh := s.view.session.cache.parseGoHandle(ctx, fh, mode) + pgh := s.parseGoHandle(ctx, fh, mode) pgf, _, err := s.parseGo(ctx, pgh) return pgf, err } @@ -107,18 +111,14 @@ func (s *snapshot) PosToDecl(ctx context.Context, pgf *source.ParsedGoFile) (map return nil, err } - pgh := s.view.session.cache.parseGoHandle(ctx, fh, pgf.Mode) + pgh := s.parseGoHandle(ctx, fh, pgf.Mode) d, err := pgh.astCacheHandle.Get(ctx, s) if err != nil { return nil, err } data := d.(*astCacheData) - if data.err != nil { - return nil, data.err - } - - return data.posToDecl, nil + return data.posToDecl, data.err } func (s *snapshot) PosToField(ctx context.Context, pgf *source.ParsedGoFile) (map[token.Pos]*ast.Field, error) { @@ -127,17 +127,14 @@ func (s *snapshot) PosToField(ctx context.Context, pgf *source.ParsedGoFile) (ma return nil, err } - pgh := s.view.session.cache.parseGoHandle(ctx, fh, pgf.Mode) + pgh := s.parseGoHandle(ctx, fh, pgf.Mode) d, err := pgh.astCacheHandle.Get(ctx, s) if err != nil || d == nil { return nil, err } data := d.(*astCacheData) - if data.err != nil { - return nil, data.err - } - return data.posToField, nil + return data.posToField, data.err } type astCacheData struct { @@ -317,6 +314,7 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod Converter: span.NewTokenConverter(fset, tok), Content: buf, } + return &parseGoData{ parsed: &source.ParsedGoFile{ URI: fh.URI(), diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 37115f6f15..a94adfe265 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -178,6 +178,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, ids: make(map[span.URI][]packageID), metadata: make(map[packageID]*metadata), files: make(map[span.URI]source.VersionedFileHandle), + goFiles: make(map[parseKey]*parseGoHandle), importedBy: make(map[packageID][]packageID), actions: make(map[actionKey]*actionHandle), workspacePackages: make(map[packageID]packagePath), diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 217e5a8cb6..6bd8296aed 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "go/ast" + "go/parser" "go/token" "go/types" "io" @@ -60,6 +61,9 @@ type snapshot struct { // It may invalidated when a file's content changes. files map[span.URI]source.VersionedFileHandle + // goFiles maps a parseKey to its parseGoHandle. + goFiles map[parseKey]*parseGoHandle + // packages maps a packageKey to a set of packageHandles to which that file belongs. // It may be invalidated when a file's content changes. packages map[packageKey]*packageHandle @@ -317,6 +321,22 @@ func (s *snapshot) transitiveReverseDependencies(id packageID, ids map[packageID } } +func (s *snapshot) getGoFile(key parseKey) *parseGoHandle { + s.mu.Lock() + defer s.mu.Unlock() + return s.goFiles[key] +} + +func (s *snapshot) addGoFile(key parseKey, pgh *parseGoHandle) *parseGoHandle { + s.mu.Lock() + defer s.mu.Unlock() + if existing, ok := s.goFiles[key]; ok { + return existing + } + s.goFiles[key] = pgh + return pgh +} + func (s *snapshot) getModHandle(uri span.URI) *parseModHandle { s.mu.Lock() defer s.mu.Unlock() @@ -763,6 +783,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve packages: make(map[packageKey]*packageHandle), actions: make(map[actionKey]*actionHandle), files: make(map[span.URI]source.VersionedFileHandle), + goFiles: make(map[parseKey]*parseGoHandle), workspacePackages: make(map[packageID]packagePath), unloadableFiles: make(map[span.URI]struct{}), parseModHandles: make(map[span.URI]*parseModHandle), @@ -784,6 +805,13 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve result.parseModHandles[k] = v } + for k, v := range s.goFiles { + if _, ok := withoutURIs[k.file.URI]; ok { + continue + } + result.goFiles[k] = v + } + // transitiveIDs keeps track of transitive reverse dependencies. // If an ID is present in the map, invalidate its types. // If an ID's value is true, invalidate its metadata too. @@ -964,27 +992,36 @@ func (s *snapshot) shouldInvalidateMetadata(ctx context.Context, originalFH, cur return originalFH.URI() == s.view.modURI } // Get the original and current parsed files in order to check package name and imports. - original, originalErr := s.ParseGo(ctx, originalFH, source.ParseHeader) - current, currentErr := s.ParseGo(ctx, currentFH, source.ParseHeader) + // Use the direct parsing API to avoid modifying the snapshot we're cloning. + parse := func(fh source.FileHandle) (*ast.File, error) { + data, err := fh.Read() + if err != nil { + return nil, err + } + fset := token.NewFileSet() + return parser.ParseFile(fset, fh.URI().Filename(), data, parser.ImportsOnly) + } + original, originalErr := parse(originalFH) + current, currentErr := parse(currentFH) if originalErr != nil || currentErr != nil { return (originalErr == nil) != (currentErr == nil) } // Check if the package's metadata has changed. The cases handled are: // 1. A package's name has changed // 2. A file's imports have changed - if original.File.Name.Name != current.File.Name.Name { + if original.Name.Name != current.Name.Name { return true } // If the package's imports have increased, definitely re-run `go list`. - if len(original.File.Imports) < len(current.File.Imports) { + if len(original.Imports) < len(current.Imports) { return true } importSet := make(map[string]struct{}) - for _, importSpec := range original.File.Imports { + for _, importSpec := range original.Imports { importSet[importSpec.Path.Value] = struct{}{} } // If any of the current imports were not in the original imports. - for _, importSpec := range current.File.Imports { + for _, importSpec := range current.Imports { if _, ok := importSet[importSpec.Path.Value]; !ok { return true } @@ -1021,7 +1058,7 @@ func (s *snapshot) buildBuiltinPackage(ctx context.Context, goFiles []string) er h := s.view.session.cache.store.Bind(fh.FileIdentity(), func(ctx context.Context, arg memoize.Arg) interface{} { snapshot := arg.(*snapshot) - pgh := snapshot.view.session.cache.parseGoHandle(ctx, fh, source.ParseFull) + pgh := snapshot.parseGoHandle(ctx, fh, source.ParseFull) pgf, _, err := snapshot.parseGo(ctx, pgh) if err != nil { return &builtinPackageData{err: err}