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 <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index f93247a..7ad3255 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -89,6 +89,8 @@
// It may invalidated when a file's content changes.
files filesMap
+ // parseCache holds an LRU cache of recently parsed files.
+ parseCache *parseCache
// parsedGoFiles maps a parseKey to the handle of the future result of parsing it.
parsedGoFiles *persistent.Map // from parseKey to *memoize.Promise[parseGoResult]
@@ -1667,6 +1669,7 @@
isActivePackageCache: s.isActivePackageCache.Clone(),
analyses: s.analyses.Clone(),
files: s.files.Clone(),
+ parseCache: s.parseCache,
parsedGoFiles: s.parsedGoFiles.Clone(),
parseKeysByURI: s.parseKeysByURI.Clone(),
symbolizeHandles: s.symbolizeHandles.Clone(),
@@ -2084,19 +2087,22 @@
}
// Parse headers to compare package names and imports.
- oldHead, oldErr := peekOrParse(ctx, lockedSnapshot, oldFH, source.ParseHeader)
- newHead, newErr := peekOrParse(ctx, lockedSnapshot, newFH, source.ParseHeader)
+ oldHeads, _, oldErr := lockedSnapshot.parseCache.parseFiles(ctx, source.ParseHeader, oldFH)
+ newHeads, _, newErr := lockedSnapshot.parseCache.parseFiles(ctx, source.ParseHeader, newFH)
if oldErr != nil || newErr != nil {
- // TODO(rfindley): we can get here if newFH does not exists. There is
- // asymmetry here, in that newFH may be non-nil even if the underlying file
- // does not exist.
+ // TODO(rfindley): we can get here if newFH does not exist. There is
+ // asymmetry, in that newFH may be non-nil even if the underlying file does
+ // not exist.
//
// We should not produce a non-nil filehandle for a file that does not exist.
errChanged := (oldErr == nil) != (newErr == nil)
return errChanged, errChanged, (newErr != nil) // we don't know if an import was deleted
}
+ oldHead := oldHeads[0]
+ newHead := newHeads[0]
+
// `go list` fails completely if the file header cannot be parsed. If we go
// from a non-parsing state to a parsing state, we should reload.
if oldHead.ParseErr != nil && newHead.ParseErr == nil {
@@ -2133,10 +2139,10 @@
// Note: if this affects performance we can probably avoid parsing in the
// common case by first scanning the source for potential comments.
if !invalidate {
- origFull, oldErr := peekOrParse(ctx, lockedSnapshot, oldFH, source.ParseFull)
- currFull, newErr := peekOrParse(ctx, lockedSnapshot, newFH, source.ParseFull)
+ origFulls, _, oldErr := lockedSnapshot.parseCache.parseFiles(ctx, source.ParseFull, oldFH)
+ newFulls, _, newErr := lockedSnapshot.parseCache.parseFiles(ctx, source.ParseFull, newFH)
if oldErr == nil && newErr == nil {
- invalidate = magicCommentsChanged(origFull.File, currFull.File)
+ invalidate = magicCommentsChanged(origFulls[0].File, newFulls[0].File)
} else {
// At this point, we shouldn't ever fail to produce a ParsedGoFile, as
// we're already past header parsing.
@@ -2147,22 +2153,6 @@
return invalidate, pkgFileChanged, importDeleted
}
-// peekOrParse returns the cached ParsedGoFile if it exists,
-// otherwise parses without populating the cache.
-//
-// It returns an error if the file could not be read (note that parsing errors
-// are stored in ParsedGoFile.ParseErr).
-//
-// lockedSnapshot must be locked.
-func peekOrParse(ctx context.Context, lockedSnapshot *snapshot, fh source.FileHandle, mode source.ParseMode) (*source.ParsedGoFile, error) {
- // Peek in the cache without populating it.
- // We do this to reduce retained heap, not work.
- if parsed, _ := lockedSnapshot.peekParseGoLocked(fh, mode); parsed != nil {
- return parsed, nil // cache hit
- }
- return parseGoImpl(ctx, token.NewFileSet(), fh, mode)
-}
-
func magicCommentsChanged(original *ast.File, current *ast.File) bool {
oldComments := extractMagicComments(original)
newComments := extractMagicComments(current)
