gopls/internal/lsp/cache: compute xrefs and methodsets asynchronously
No need to wait on xrefs or methodsets while performing
latency-sensitive operations on packages.
For golang/go#53992
Change-Id: I9ea65269a8c1e604fb99ed8b25e14db79f179576
Reviewed-on: https://go-review.googlesource.com/c/tools/+/503015
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go
index 5f5400b..6cfabe3 100644
--- a/gopls/internal/lsp/cache/check.go
+++ b/gopls/internal/lsp/cache/check.go
@@ -26,9 +26,7 @@
"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"
"golang.org/x/tools/gopls/internal/lsp/source/typerefs"
- "golang.org/x/tools/gopls/internal/lsp/source/xrefs"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/event/tag"
@@ -44,6 +42,8 @@
preserveImportGraph = true // hold on to the import graph for open packages
)
+type unit = struct{}
+
// A typeCheckBatch holds data for a logical type-checking operation, which may
// type-check many unrelated packages.
//
@@ -56,7 +56,7 @@
handles map[PackageID]*packageHandle
parseCache *parseCache
fset *token.FileSet // describes all parsed or imported files
- cpulimit chan struct{} // concurrency limiter for CPU-bound operations
+ cpulimit chan unit // concurrency limiter for CPU-bound operations
mu sync.Mutex
syntaxPackages map[PackageID]*futurePackage // results of processing a requested package; may hold (nil, nil)
@@ -69,7 +69,7 @@
// The goroutine that creates the futurePackage is responsible for evaluating
// its value, and closing the done channel.
type futurePackage struct {
- done chan struct{}
+ done chan unit
v pkgOrErr
}
@@ -154,7 +154,7 @@
// for the work to be done.
done := s.importGraphDone
if done == nil {
- done = make(chan struct{})
+ done = make(chan unit)
s.importGraphDone = done
release := s.Acquire() // must acquire to use the snapshot asynchronously
go func() {
@@ -360,7 +360,7 @@
handles: handles,
fset: fileSetWithBase(reservedForParsing),
syntaxIndex: make(map[PackageID]int),
- cpulimit: make(chan struct{}, runtime.GOMAXPROCS(0)),
+ cpulimit: make(chan unit, runtime.GOMAXPROCS(0)),
syntaxPackages: make(map[PackageID]*futurePackage),
importPackages: make(map[PackageID]*futurePackage),
}
@@ -369,7 +369,7 @@
// Clone the file set every time, to ensure we do not leak files.
b.fset = tokeninternal.CloneFileSet(importGraph.fset)
// Pre-populate future cache with 'done' futures.
- done := make(chan struct{})
+ done := make(chan unit)
close(done)
for id, res := range importGraph.imports {
b.importPackages[id] = &futurePackage{done, res}
@@ -427,7 +427,7 @@
}
}
- f = &futurePackage{done: make(chan struct{})}
+ f = &futurePackage{done: make(chan unit)}
b.importPackages[id] = f
b.mu.Unlock()
@@ -479,7 +479,7 @@
return f.v.pkg, f.v.err
}
- f = &futurePackage{done: make(chan struct{})}
+ f = &futurePackage{done: make(chan unit)}
b.syntaxPackages[id] = f
b.mu.Unlock()
defer func() {
@@ -508,7 +508,7 @@
select {
case <-ctx.Done():
return nil, ctx.Err()
- case b.cpulimit <- struct{}{}:
+ case b.cpulimit <- unit{}:
defer func() {
<-b.cpulimit // release CPU token
}()
@@ -637,8 +637,8 @@
// Write package data to disk asynchronously.
go func() {
toCache := map[string][]byte{
- xrefsKind: pkg.xrefs,
- methodSetsKind: pkg.methodsets.Encode(),
+ xrefsKind: pkg.xrefs(),
+ methodSetsKind: pkg.methodsets().Encode(),
diagnosticsKind: encodeDiagnostics(pkg.diagnostics),
}
@@ -763,13 +763,13 @@
// Collect all reachable IDs, and create done channels.
// TODO: opt: modify SortPostOrder to make this pre-traversal unnecessary.
var allIDs []PackageID
- dones := make(map[PackageID]chan struct{})
+ dones := make(map[PackageID]chan unit)
var walk func(PackageID)
walk = func(id PackageID) {
if _, ok := dones[id]; ok {
return
}
- dones[id] = make(chan struct{})
+ dones[id] = make(chan unit)
allIDs = append(allIDs, id)
m := meta.metadata[id]
for _, depID := range m.DepsByPkgPath {
@@ -1262,8 +1262,6 @@
if err != nil {
return nil, err
}
- pkg.methodsets = methodsets.NewIndex(pkg.fset, pkg.types)
- pkg.xrefs = xrefs.Index(pkg.compiledGoFiles, pkg.types, pkg.typesInfo)
// Our heuristic for whether to show type checking errors is:
// + If any file was 'fixed', don't show type checking errors as we
diff --git a/gopls/internal/lsp/cache/pkg.go b/gopls/internal/lsp/cache/pkg.go
index dbbb4be..32f6349 100644
--- a/gopls/internal/lsp/cache/pkg.go
+++ b/gopls/internal/lsp/cache/pkg.go
@@ -11,9 +11,11 @@
"go/scanner"
"go/token"
"go/types"
+ "sync"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/lsp/source/methodsets"
+ "golang.org/x/tools/gopls/internal/lsp/source/xrefs"
"golang.org/x/tools/gopls/internal/span"
)
@@ -53,11 +55,25 @@
importMap map[PackagePath]*types.Package
hasFixedFiles bool // if true, AST was sufficiently mangled that we should hide type errors
- // TODO(rfindley): opt: xrefs and methodsets do not need to be pinned to the
- // package, and perhaps should be computed asynchronously to package
- // diagnostics.
- xrefs []byte
- methodsets *methodsets.Index
+ xrefsOnce sync.Once
+ _xrefs []byte // only used by the xrefs method
+
+ methodsetsOnce sync.Once
+ _methodsets *methodsets.Index // only used by the methodsets method
+}
+
+func (p *syntaxPackage) xrefs() []byte {
+ p.xrefsOnce.Do(func() {
+ p._xrefs = xrefs.Index(p.compiledGoFiles, p.types, p.typesInfo)
+ })
+ return p._xrefs
+}
+
+func (p *syntaxPackage) methodsets() *methodsets.Index {
+ p.methodsetsOnce.Do(func() {
+ p._methodsets = methodsets.NewIndex(p.fset, p.types)
+ })
+ return p._methodsets
}
func (p *Package) String() string { return string(p.ph.m.ID) }
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index bff3dc1..6ff14fb 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -688,7 +688,7 @@
return true
}
post := func(i int, pkg *Package) {
- indexes[i] = XrefIndex{m: pkg.ph.m, data: pkg.pkg.xrefs}
+ indexes[i] = XrefIndex{m: pkg.ph.m, data: pkg.pkg.xrefs()}
}
return indexes, s.forEachPackage(ctx, ids, pre, post)
}
@@ -719,7 +719,7 @@
return true
}
post := func(i int, pkg *Package) {
- indexes[i] = pkg.pkg.methodsets
+ indexes[i] = pkg.pkg.methodsets()
}
return indexes, s.forEachPackage(ctx, ids, pre, post)
}
