internal/lsp/cache: add an LRU parse cache

As work proceeds on incremental type-checking, two observations have
emerged from benchmarking:
- Using a global FileSet is impossible, as IImportShallow allocates a
  large number of new token.Files (in early experiments 75%+ of in-use memory
  was consumed by the FileSet!)
- Several benchmarks regressed with incremental type-checking due to
  re-parsing package files following a change. Ideally after a single file
  changes we would be able to re-typecheck packages containing that file
  after only re-parsing the single file that changed.

These observations are in tension: because type-checking requires that
parsed ast.Files live in the same token.FileSet as the type-checked
package, we cannot naively save the results of parsing and still use a
package-scoped FileSet.

This CL seeks to address both observations, by introducing a new
mechanism for caching parsed files (a parseCache) that parses files in a
standalone FileSet offset to avoid collision with other parsed files.
This cache exposes a batch API to parse multiple files and return a
FileSet describing all of them.

Benchmarking indicates that this partially mitigates performance
regressions without sacrificing the memory improvement we by avoiding a
global cache of parsed files.

In this CL the parse cache is not yet integrated with type-checking, but
replaces certain call-sites where we previously tried to avoid parsing
through the cache.

For golang/go#57987

Change-Id: I840cf003db835a40721f086abcc7bf00486b8108
Reviewed-on: https://go-review.googlesource.com/c/tools/+/469858
Reviewed-by: Alan Donovan <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
gopls-CI: kokoro <[email protected]>
Run-TryBot: Robert Findley <[email protected]>

Author: findleyr

gopls/internal/lsp/cache/check.go

@@ -677,7 +677,7 @@ func depsErrors(ctx context.Context, m *source.Metadata, meta *metadataGraph, fs
 		if err != nil {
 			return nil, err
 		}
-		fset := source.SingletonFileSet(pgf.Tok)
+		fset := source.FileSetFor(pgf.Tok)
 		// TODO(adonovan): modify Imports() to accept a single token.File (cgf.Tok).
 		for _, group := range astutil.Imports(fset, pgf.File) {
 			for _, imp := range group {

gopls/internal/lsp/cache/mod_tidy.go

@@ -203,7 +203,11 @@ func modTidyDiagnostics(ctx context.Context, snapshot *snapshot, pm *source.Pars
 		// If -mod=readonly is not set we may have successfully imported
 		// packages from missing modules. Otherwise they'll be in
 		// MissingDependencies. Combine both.
-		for imp := range parseImports(ctx, snapshot, goFiles) {
+		imps, err := parseImports(ctx, snapshot, goFiles)
+		if err != nil {
+			return nil, err
+		}
+		for imp := range imps {
 			if req, ok := missing[imp]; ok {
 				missingImports[imp] = req
 				break
@@ -446,19 +450,20 @@ func missingModuleForImport(pgf *source.ParsedGoFile, imp *ast.ImportSpec, req *
 //
 // (We can't simply use Metadata.Imports because it is based on
 // CompiledGoFiles, after cgo processing.)
-func parseImports(ctx context.Context, s *snapshot, files []source.FileHandle) map[string]bool {
-	s.mu.Lock() // peekOrParse requires a locked snapshot (!)
-	defer s.mu.Unlock()
+//
+// TODO(rfindley): this should key off source.ImportPath.
+func parseImports(ctx context.Context, s *snapshot, files []source.FileHandle) (map[string]bool, error) {
+	pgfs, _, err := s.parseCache.parseFiles(ctx, source.ParseHeader, files...)
+	if err != nil { // e.g. context cancellation
+		return nil, err
+	}
+
 	seen := make(map[string]bool)
-	for _, file := range files {
-		f, err := peekOrParse(ctx, s, file, source.ParseHeader)
-		if err != nil {
-			continue
-		}
-		for _, spec := range f.File.Imports {
+	for _, pgf := range pgfs {
+		for _, spec := range pgf.File.Imports {
 			path, _ := strconv.Unquote(spec.Path.Value)
 			seen[path] = true
 		}
 	}
-	return seen
+	return seen, nil
 }

gopls/internal/lsp/cache/parse.go

@@ -21,18 +21,13 @@ import (
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
 	"golang.org/x/tools/gopls/internal/lsp/safetoken"
 	"golang.org/x/tools/gopls/internal/lsp/source"
+	"golang.org/x/tools/gopls/internal/span"
 	"golang.org/x/tools/internal/diff"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/event/tag"
 	"golang.org/x/tools/internal/memoize"
 )
 
-// parseKey uniquely identifies a parsed Go file.
-type parseKey struct {
-	file source.FileIdentity
-	mode source.ParseMode
-}
-
 // ParseGo parses the file whose contents are provided by fh, using a cache.
 // The resulting tree may have be fixed up.
 //
@@ -111,22 +106,6 @@ func (s *snapshot) ParseGo(ctx context.Context, fh source.FileHandle, mode sourc
 	return res.parsed, res.err
 }
 
-// peekParseGoLocked peeks at the cache used by ParseGo but does not
-// populate it or wait for other threads to do so. On cache hit, it returns
-// the cache result of parseGoImpl; otherwise it returns (nil, nil).
-func (s *snapshot) peekParseGoLocked(fh source.FileHandle, mode source.ParseMode) (*source.ParsedGoFile, error) {
-	entry, hit := s.parsedGoFiles.Get(parseKey{fh.FileIdentity(), mode})
-	if !hit {
-		return nil, nil // no-one has requested this file
-	}
-	v := entry.(*memoize.Promise).Cached()
-	if v == nil {
-		return nil, nil // parsing is still in progress
-	}
-	res := v.(parseGoResult)
-	return res.parsed, res.err
-}
-
 // parseGoResult holds the result of a call to parseGoImpl.
 type parseGoResult struct {
 	parsed *source.ParsedGoFile
@@ -146,13 +125,17 @@ func parseGoImpl(ctx context.Context, fset *token.FileSet, fh source.FileHandle,
 	if err != nil {
 		return nil, err
 	}
+	return parseGoSrc(ctx, fset, fh.URI(), src, mode), nil
+}
 
+// parseGoSrc parses a buffer of Go source, repairing the tree if necessary.
+func parseGoSrc(ctx context.Context, fset *token.FileSet, uri span.URI, src []byte, mode source.ParseMode) (res *source.ParsedGoFile) {
 	parserMode := parser.AllErrors | parser.ParseComments
 	if mode == source.ParseHeader {
 		parserMode = parser.ImportsOnly | parser.ParseComments
 	}
 
-	file, err := parser.ParseFile(fset, fh.URI().Filename(), src, parserMode)
+	file, err := parser.ParseFile(fset, uri.Filename(), src, parserMode)
 	var parseErr scanner.ErrorList
 	if err != nil {
 		// We passed a byte slice, so the only possible error is a parse error.
@@ -164,7 +147,7 @@ func parseGoImpl(ctx context.Context, fset *token.FileSet, fh source.FileHandle,
 		// file.Pos is the location of the package declaration (issue #53202). If there was
 		// none, we can't find the token.File that ParseFile created, and we
 		// have no choice but to recreate it.
-		tok = fset.AddFile(fh.URI().Filename(), -1, len(src))
+		tok = fset.AddFile(uri.Filename(), -1, len(src))
 		tok.SetLinesForContent(src)
 	}
 
@@ -189,7 +172,7 @@ func parseGoImpl(ctx context.Context, fset *token.FileSet, fh source.FileHandle,
 				event.Log(ctx, fmt.Sprintf("fixSrc loop - last diff:\n%v", unified), tag.File.Of(tok.Name()))
 			}
 
-			newFile, _ := parser.ParseFile(fset, fh.URI().Filename(), newSrc, parserMode)
+			newFile, _ := parser.ParseFile(fset, uri.Filename(), newSrc, parserMode)
 			if newFile != nil {
 				// Maintain the original parseError so we don't try formatting the doctored file.
 				file = newFile
@@ -202,15 +185,15 @@ func parseGoImpl(ctx context.Context, fset *token.FileSet, fh source.FileHandle,
 	}
 
 	return &source.ParsedGoFile{
-		URI:      fh.URI(),
+		URI:      uri,
 		Mode:     mode,
 		Src:      src,
 		Fixed:    fixed,
 		File:     file,
 		Tok:      tok,
-		Mapper:   protocol.NewMapper(fh.URI(), src),
+		Mapper:   protocol.NewMapper(uri, src),
 		ParseErr: parseErr,
-	}, nil
+	}
 }
 
 // An unexportedFilter removes as much unexported AST from a set of Files as possible.