internal/lsp/cache: follow usual structure for packages, analysis maps

All Get/Set operations on the maps now happen within a single
function (buildPackageKey, actionHandle).

No behavior change.

Change-Id: I347dfda578c28657a28538e228ecfb6f0871b94b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417415
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
This commit is contained in:
Alan Donovan 2022-07-13 13:46:53 -04:00 committed by Gopher Robot
parent b2eae76267
commit 85173cc4bd
4 changed files with 83 additions and 133 deletions

View File

@ -58,6 +58,11 @@ func (s *snapshot) Analyze(ctx context.Context, id string, analyzers []*source.A
return results, nil
}
type actionKey struct {
pkg packageKey
analyzer *analysis.Analyzer
}
type actionHandleKey source.Hash
// An action represents one unit of analysis work: the application of
@ -90,6 +95,20 @@ type packageFactKey struct {
}
func (s *snapshot) actionHandle(ctx context.Context, id PackageID, a *analysis.Analyzer) (*actionHandle, error) {
const mode = source.ParseFull
key := actionKey{
pkg: packageKey{id: id, mode: mode},
analyzer: a,
}
s.mu.Lock()
entry, hit := s.actions.Get(key)
s.mu.Unlock()
if hit {
return entry.(*actionHandle), nil
}
// TODO(adonovan): opt: this block of code sequentially loads a package
// (and all its dependencies), then sequentially creates action handles
// for the direct dependencies (whose packages have by then been loaded
@ -100,15 +119,6 @@ func (s *snapshot) actionHandle(ctx context.Context, id PackageID, a *analysis.A
// use of concurrency would lead to an exponential amount of duplicated
// work. We should instead use an atomically updated future cache
// and a parallel graph traversal.
// TODO(adonovan): in the code below, follow the structure of
// the other handle-map accessors.
const mode = source.ParseFull
if act := s.getActionHandle(id, mode, a); act != nil {
return act, nil
}
ph, err := s.buildPackageHandle(ctx, id, mode)
if err != nil {
return nil, err
@ -157,13 +167,24 @@ func (s *snapshot) actionHandle(ctx context.Context, id PackageID, a *analysis.A
return runAnalysis(ctx, snapshot, a, pkg, results)
})
act := &actionHandle{
ah := &actionHandle{
analyzer: a,
pkg: pkg,
handle: handle,
}
act = s.addActionHandle(act, release)
return act, nil
s.mu.Lock()
defer s.mu.Unlock()
// Check cache again in case another thread got there first.
if result, ok := s.actions.Get(key); ok {
release()
return result.(*actionHandle), nil
}
s.actions.Set(key, ah, func(_, _ interface{}) { release() })
return ah, nil
}
func (act *actionHandle) analyze(ctx context.Context, snapshot *snapshot) ([]*source.Diagnostic, interface{}, error) {

View File

@ -199,7 +199,7 @@ func (c *Cache) PackageStats(withNames bool) template.HTML {
c.store.DebugOnlyIterate(func(k, v interface{}) {
switch k.(type) {
case packageHandleKey:
v := v.(*packageData)
v := v.(typeCheckResult)
if v.pkg == nil {
break
}

View File

@ -22,6 +22,7 @@ import (
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/lsp/bug"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
@ -43,10 +44,7 @@ type packageHandleKey source.Hash
// A packageHandle is a handle to the future result of type-checking a package.
// The resulting package is obtained from the check() method.
type packageHandle struct {
handle *memoize.Handle
// mode is the mode the files will be parsed in.
mode source.ParseMode
handle *memoize.Handle // [typeCheckResult]
// m is the metadata associated with the package.
m *KnownMetadata
@ -61,15 +59,9 @@ type packageHandle struct {
key packageHandleKey
}
func (ph *packageHandle) packageKey() packageKey {
return packageKey{
id: ph.m.ID,
mode: ph.mode,
}
}
// packageData contains the data produced by type-checking a package.
type packageData struct {
// typeCheckResult contains the result of a call to
// typeCheck, which type-checks a package.
type typeCheckResult struct {
pkg *pkg
err error
}
@ -80,15 +72,21 @@ type packageData struct {
// attempt to reload missing or invalid metadata. The caller must reload
// metadata if needed.
func (s *snapshot) buildPackageHandle(ctx context.Context, id PackageID, mode source.ParseMode) (*packageHandle, error) {
if ph := s.getPackage(id, mode); ph != nil {
return ph, nil
}
packageKey := packageKey{id: id, mode: mode}
s.mu.Lock()
entry, hit := s.packages.Get(packageKey)
m := s.meta.metadata[id]
s.mu.Unlock()
m := s.getMetadata(id)
if m == nil {
return nil, fmt.Errorf("no metadata for %s", id)
}
if hit {
return entry.(*packageHandle), nil
}
// Begin computing the key by getting the depKeys for all dependencies.
// This requires reading the transitive closure of dependencies' source files.
//
@ -140,10 +138,10 @@ func (s *snapshot) buildPackageHandle(ctx context.Context, id PackageID, mode so
// All the file reading has now been done.
// Create a handle for the result of type checking.
experimentalKey := s.View().Options().ExperimentalPackageCacheKey
key := computePackageKey(m.ID, compiledGoFiles, m, depKeys, mode, experimentalKey)
phKey := computePackageKey(m.ID, compiledGoFiles, m, depKeys, mode, experimentalKey)
// TODO(adonovan): extract lambda into a standalone function to
// avoid implicit lexical dependencies.
handle, release := s.store.Handle(key, func(ctx context.Context, arg interface{}) interface{} {
handle, release := s.store.Handle(phKey, func(ctx context.Context, arg interface{}) interface{} {
snapshot := arg.(*snapshot)
// Start type checking of direct dependencies,
@ -167,21 +165,43 @@ func (s *snapshot) buildPackageHandle(ctx context.Context, id PackageID, mode so
defer wg.Wait()
pkg, err := typeCheck(ctx, snapshot, goFiles, compiledGoFiles, m.Metadata, mode, deps)
return &packageData{pkg, err}
return typeCheckResult{pkg, err}
})
ph := &packageHandle{
handle: handle,
mode: mode,
m: m,
key: key,
key: phKey,
}
// Cache the handle in the snapshot. If a package handle has already
// been cached, addPackage will return the cached value. This is fine,
// since the original package handle above will have no references and be
// garbage collected.
return s.addPackageHandle(ph, release)
s.mu.Lock()
defer s.mu.Unlock()
// Check that the metadata has not changed
// (which should invalidate this handle).
//
// (In future, handles should form a graph with edges from a
// packageHandle to the handles for parsing its files and the
// handles for type-checking its immediate deps, at which
// point there will be no need to even access s.meta.)
if s.meta.metadata[ph.m.ID].Metadata != ph.m.Metadata {
return nil, fmt.Errorf("stale metadata for %s", ph.m.ID)
}
// Check cache again in case another thread got there first.
if prev, ok := s.packages.Get(packageKey); ok {
prevPH := prev.(*packageHandle)
release()
if prevPH.m.Metadata != ph.m.Metadata {
return nil, bug.Errorf("existing package handle does not match for %s", ph.m.ID)
}
return prevPH, nil
}
// Update the map.
s.packages.Set(packageKey, ph, func(_, _ interface{}) { release() })
return ph, nil
}
// readGoFiles reads the content of Metadata.GoFiles and
@ -273,7 +293,7 @@ func (ph *packageHandle) check(ctx context.Context, s *snapshot) (*pkg, error) {
if err != nil {
return nil, err
}
data := v.(*packageData)
data := v.(typeCheckResult)
return data.pkg, data.err
}
@ -290,7 +310,7 @@ func (ph *packageHandle) cached() (*pkg, error) {
if v == nil {
return nil, fmt.Errorf("no cached type information for %s", ph.m.PkgPath)
}
data := v.(*packageData)
data := v.(typeCheckResult)
return data.pkg, data.err
}

View File

@ -30,7 +30,6 @@ import (
"golang.org/x/mod/module"
"golang.org/x/mod/semver"
"golang.org/x/sync/errgroup"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/gocommand"
@ -175,11 +174,6 @@ func (s *snapshot) awaitHandle(ctx context.Context, h *memoize.Handle) (interfac
return h.Get(ctx, s)
}
type actionKey struct {
pkg packageKey
analyzer *analysis.Analyzer
}
// destroy waits for all leases on the snapshot to expire then releases
// any resources (reference counts and files) associated with it.
// Snapshots being destroyed can be awaited using v.destroyWG.
@ -771,34 +765,6 @@ func (s *snapshot) getImportedBy(id PackageID) []PackageID {
return s.meta.importedBy[id]
}
// addPackageHandle stores ph in the snapshot, or returns a pre-existing handle
// for the given package key, if it exists.
//
// An error is returned if the metadata used to build ph is no longer relevant.
//
// TODO(adonovan): inline sole use in buildPackageHandle.
func (s *snapshot) addPackageHandle(ph *packageHandle, release func()) (*packageHandle, error) {
s.mu.Lock()
defer s.mu.Unlock()
if s.meta.metadata[ph.m.ID].Metadata != ph.m.Metadata {
return nil, fmt.Errorf("stale metadata for %s", ph.m.ID)
}
// If the package handle has already been cached,
// return the cached handle instead of overriding it.
if v, ok := s.packages.Get(ph.packageKey()); ok {
result := v.(*packageHandle)
release()
if result.m.Metadata != ph.m.Metadata {
return nil, bug.Errorf("existing package handle does not match for %s", ph.m.ID)
}
return result, nil
}
s.packages.Set(ph.packageKey(), ph, func(_, _ interface{}) { release() })
return ph, nil
}
func (s *snapshot) workspacePackageIDs() (ids []PackageID) {
s.mu.Lock()
defer s.mu.Unlock()
@ -1202,63 +1168,6 @@ func moduleForURI(modFiles map[span.URI]struct{}, uri span.URI) span.URI {
return match
}
// TODO(adonovan): inline sole use in buildPackageHandle.
func (s *snapshot) getPackage(id PackageID, mode source.ParseMode) *packageHandle {
s.mu.Lock()
defer s.mu.Unlock()
key := packageKey{
id: id,
mode: mode,
}
v, ok := s.packages.Get(key)
if !ok {
return nil
}
return v.(*packageHandle)
}
func (s *snapshot) getActionHandle(id PackageID, m source.ParseMode, a *analysis.Analyzer) *actionHandle {
key := actionKey{
pkg: packageKey{
id: id,
mode: m,
},
analyzer: a,
}
s.mu.Lock()
defer s.mu.Unlock()
ah, ok := s.actions.Get(key)
if !ok {
return nil
}
return ah.(*actionHandle)
}
func (s *snapshot) addActionHandle(ah *actionHandle, release func()) *actionHandle {
key := actionKey{
analyzer: ah.analyzer,
pkg: packageKey{
id: ah.pkg.m.ID,
mode: ah.pkg.mode,
},
}
s.mu.Lock()
defer s.mu.Unlock()
// If another thread since cached a different handle,
// return it instead of overriding it.
if result, ok := s.actions.Get(key); ok {
release()
return result.(*actionHandle)
}
s.actions.Set(key, ah, func(_, _ interface{}) { release() })
return ah
}
func (s *snapshot) getIDsForURI(uri span.URI) []PackageID {
s.mu.Lock()
defer s.mu.Unlock()