Skip to content

Commit

Permalink
internal/lsp/cache: add an LRU parse cache
Browse files Browse the repository at this point in the history
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 <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
  • Loading branch information
findleyr committed Mar 3, 2023
1 parent de54582 commit ae05609
Show file tree
Hide file tree
Showing 14 changed files with 509 additions and 119 deletions.
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cache/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
27 changes: 16 additions & 11 deletions gopls/internal/lsp/cache/mod_tidy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
39 changes: 11 additions & 28 deletions gopls/internal/lsp/cache/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand 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
Expand All @@ -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.
Expand All @@ -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)
}

Expand All @@ -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
Expand All @@ -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.
Expand Down
Loading

0 comments on commit ae05609

Please sign in to comment.