gopls/internal/lsp/cache: stream package results
As we move toward incremental evaluation of the package key and precise
pruning, rewrite the type checking pass to formalize the concept of a
pre- and post- package visitor. Whereas previously we would do an
initial pass outside of type-checking to load data (xrefs, diagnostics,
etc), we now integrate loads checks with the batch operation, calling
a pre- func immediately before type-checking, and a post- func
immediately afterward.
This change was originally intended only to move us toward precise
pruning, but inadvertently simplified and clarified many aspects of
type-checking. Namely, it:
- resulted in eliminating all state from the type check batch;
typeCheckBatch.mu is deleted
- allows avoiding double-encoding of diagnostics and method sets, to fit
the previous APIs. This results in a significant improvement in the
methodsets benchmarks
- significantly simplifies the logic of the type-checking batch
- significantly reduces the high water mark during the IWL, as the GC
can clean up the packages that are no longer pinned to the batch
For golang/go#57987
Change-Id: Iad1073f32cea0a7e26567d94cdd3ed01ff60ad04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/476956
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 406a428..bf026b6 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -29,6 +29,7 @@
"golang.org/x/sync/errgroup"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/go/types/objectpath"
+ "golang.org/x/tools/gopls/internal/lsp/filecache"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/lsp/source/methodsets"
@@ -633,34 +634,49 @@
)
func (s *snapshot) PackageDiagnostics(ctx context.Context, ids ...PackageID) (map[span.URI][]*source.Diagnostic, error) {
- // TODO(rfindley): opt: avoid unnecessary encode->decode after type-checking.
- data, err := s.getPackageData(ctx, diagnosticsKind, ids, func(p *syntaxPackage) []byte {
- return encodeDiagnostics(p.diagnostics)
- })
+ var mu sync.Mutex
perFile := make(map[span.URI][]*source.Diagnostic)
- for _, data := range data {
- if data != nil {
- for _, diag := range data.m.Diagnostics {
- perFile[diag.URI] = append(perFile[diag.URI], diag)
- }
- diags := decodeDiagnostics(data.data)
- for _, diag := range diags {
- perFile[diag.URI] = append(perFile[diag.URI], diag)
- }
+ collect := func(diags []*source.Diagnostic) {
+ mu.Lock()
+ defer mu.Unlock()
+ for _, diag := range diags {
+ perFile[diag.URI] = append(perFile[diag.URI], diag)
}
}
- return perFile, err
+ pre := func(i int, ph *packageHandle) bool {
+ data, err := filecache.Get(diagnosticsKind, ph.key)
+ if err == nil { // hit
+ collect(ph.m.Diagnostics)
+ collect(decodeDiagnostics(data))
+ return false
+ } else if err != filecache.ErrNotFound {
+ event.Error(ctx, "reading diagnostics from filecache", err)
+ }
+ return true
+ }
+ post := func(_ int, pkg *Package) {
+ collect(pkg.m.Diagnostics)
+ collect(pkg.pkg.diagnostics)
+ }
+ return perFile, s.forEachPackage(ctx, ids, pre, post)
}
func (s *snapshot) References(ctx context.Context, ids ...PackageID) ([]source.XrefIndex, error) {
- data, err := s.getPackageData(ctx, xrefsKind, ids, func(p *syntaxPackage) []byte { return p.xrefs })
indexes := make([]source.XrefIndex, len(ids))
- for i, data := range data {
- if data != nil {
- indexes[i] = XrefIndex{m: data.m, data: data.data}
+ pre := func(i int, ph *packageHandle) bool {
+ data, err := filecache.Get(xrefsKind, ph.key)
+ if err == nil { // hit
+ indexes[i] = XrefIndex{m: ph.m, data: data}
+ return false
+ } else if err != filecache.ErrNotFound {
+ event.Error(ctx, "reading xrefs from filecache", err)
}
+ return true
}
- return indexes, err
+ post := func(i int, pkg *Package) {
+ indexes[i] = XrefIndex{m: pkg.m, data: pkg.pkg.xrefs}
+ }
+ return indexes, s.forEachPackage(ctx, ids, pre, post)
}
// An XrefIndex is a helper for looking up a package in a given package.
@@ -674,21 +690,21 @@
}
func (s *snapshot) MethodSets(ctx context.Context, ids ...PackageID) ([]*methodsets.Index, error) {
- // TODO(rfindley): opt: avoid unnecessary encode->decode after type-checking.
- data, err := s.getPackageData(ctx, methodSetsKind, ids, func(p *syntaxPackage) []byte {
- return p.methodsets.Encode()
- })
indexes := make([]*methodsets.Index, len(ids))
- for i, data := range data {
- if data != nil {
- indexes[i] = methodsets.Decode(data.data)
- } else if ids[i] == "unsafe" {
- indexes[i] = &methodsets.Index{}
- } else {
- panic(fmt.Sprintf("nil data for %s", ids[i]))
+ pre := func(i int, ph *packageHandle) bool {
+ data, err := filecache.Get(methodSetsKind, ph.key)
+ if err == nil { // hit
+ indexes[i] = methodsets.Decode(data)
+ return false
+ } else if err != filecache.ErrNotFound {
+ event.Error(ctx, "reading methodsets from filecache", err)
}
+ return true
}
- return indexes, err
+ post := func(i int, pkg *Package) {
+ indexes[i] = pkg.pkg.methodsets
+ }
+ return indexes, s.forEachPackage(ctx, ids, pre, post)
}
func (s *snapshot) MetadataForFile(ctx context.Context, uri span.URI) ([]*source.Metadata, error) {
