internal/lsp: use ids instead of package paths as map keys
This adds an IDs map to the metadata cache, which maps package paths to
IDs. This is only ever used by the Import function in the type checker.
Change-Id: I8677d9439895bc6cbca5072e3fa9fddad4e165d5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181683
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 99a4f67..e9ae93b 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -22,7 +22,7 @@
// seen maintains the set of previously imported packages.
// If we have seen a package that is already in this map, we have a circular import.
- seen map[packagePath]struct{}
+ seen map[packageID]struct{}
// topLevelPkgID is the ID of the package from which type-checking began.
topLevelPkgID packageID
@@ -32,19 +32,23 @@
}
func (imp *importer) Import(pkgPath string) (*types.Package, error) {
- pkg, err := imp.getPkg(packagePath(pkgPath))
+ id, ok := imp.view.mcache.ids[packagePath(pkgPath)]
+ if !ok {
+ return nil, fmt.Errorf("no known ID for %s", pkgPath)
+ }
+ pkg, err := imp.getPkg(id)
if err != nil {
return nil, err
}
return pkg.types, nil
}
-func (imp *importer) getPkg(pkgPath packagePath) (*pkg, error) {
- if _, ok := imp.seen[pkgPath]; ok {
+func (imp *importer) getPkg(id packageID) (*pkg, error) {
+ if _, ok := imp.seen[id]; ok {
return nil, fmt.Errorf("circular import detected")
}
imp.view.pcache.mu.Lock()
- e, ok := imp.view.pcache.packages[pkgPath]
+ e, ok := imp.view.pcache.packages[id]
if ok {
// cache hit
@@ -54,12 +58,12 @@
} else {
// cache miss
e = &entry{ready: make(chan struct{})}
- imp.view.pcache.packages[pkgPath] = e
+ imp.view.pcache.packages[id] = e
imp.view.pcache.mu.Unlock()
// This goroutine becomes responsible for populating
// the entry and broadcasting its readiness.
- e.pkg, e.err = imp.typeCheck(pkgPath)
+ e.pkg, e.err = imp.typeCheck(id)
close(e.ready)
}
@@ -68,11 +72,11 @@
if e.err == context.Canceled && imp.ctx.Err() == nil {
imp.view.pcache.mu.Lock()
// Clear out canceled cache entry if it is still there.
- if imp.view.pcache.packages[pkgPath] == e {
- delete(imp.view.pcache.packages, pkgPath)
+ if imp.view.pcache.packages[id] == e {
+ delete(imp.view.pcache.packages, id)
}
imp.view.pcache.mu.Unlock()
- return imp.getPkg(pkgPath)
+ return imp.getPkg(id)
}
return nil, e.err
}
@@ -80,10 +84,10 @@
return e.pkg, nil
}
-func (imp *importer) typeCheck(pkgPath packagePath) (*pkg, error) {
- meta, ok := imp.view.mcache.packages[pkgPath]
+func (imp *importer) typeCheck(id packageID) (*pkg, error) {
+ meta, ok := imp.view.mcache.packages[id]
if !ok {
- return nil, fmt.Errorf("no metadata for %v", pkgPath)
+ return nil, fmt.Errorf("no metadata for %v", id)
}
pkg := &pkg{
id: meta.id,
@@ -123,11 +127,11 @@
pkg.syntax = files
// Handle circular imports by copying previously seen imports.
- seen := make(map[packagePath]struct{})
+ seen := make(map[packageID]struct{})
for k, v := range imp.seen {
seen[k] = v
}
- seen[pkgPath] = struct{}{}
+ seen[id] = struct{}{}
cfg := &types.Config{
Error: func(err error) {
@@ -198,7 +202,7 @@
if err != nil {
continue
}
- pkg.imports[importPath] = importPkg
+ pkg.imports[importPkg.pkgPath] = importPkg
}
}
diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go
index a9dd25f..1be2ffd 100644
--- a/internal/lsp/cache/gofile.go
+++ b/internal/lsp/cache/gofile.go
@@ -131,9 +131,9 @@
f.view.mcache.mu.Lock()
defer f.view.mcache.mu.Unlock()
- seen := make(map[packagePath]struct{}) // visited packages
+ seen := make(map[packageID]struct{}) // visited packages
results := make(map[*goFile]struct{})
- f.view.reverseDeps(ctx, seen, results, packagePath(pkg.PkgPath()))
+ f.view.reverseDeps(ctx, seen, results, packageID(pkg.ID()))
var files []source.GoFile
for rd := range results {
@@ -149,12 +149,12 @@
return files
}
-func (v *view) reverseDeps(ctx context.Context, seen map[packagePath]struct{}, results map[*goFile]struct{}, pkgPath packagePath) {
- if _, ok := seen[pkgPath]; ok {
+func (v *view) reverseDeps(ctx context.Context, seen map[packageID]struct{}, results map[*goFile]struct{}, id packageID) {
+ if _, ok := seen[id]; ok {
return
}
- seen[pkgPath] = struct{}{}
- m, ok := v.mcache.packages[pkgPath]
+ seen[id] = struct{}{}
+ m, ok := v.mcache.packages[id]
if !ok {
return
}
@@ -164,7 +164,7 @@
results[f.(*goFile)] = struct{}{}
}
}
- for parentPkgPath := range m.parents {
- v.reverseDeps(ctx, seen, results, parentPkgPath)
+ for parentID := range m.parents {
+ v.reverseDeps(ctx, seen, results, parentID)
}
}
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 71da94c..c8963fd 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -18,7 +18,6 @@
if f.astIsTrimmed() {
f.invalidateAST()
}
-
// Save the metadata's current missing imports, if any.
var originalMissingImports map[packagePath]struct{}
if f.meta != nil {
@@ -37,21 +36,19 @@
if sameSet(originalMissingImports, f.meta.missingImports) && f.pkg != nil {
return nil, nil
}
-
imp := &importer{
view: v,
- seen: make(map[packagePath]struct{}),
+ seen: make(map[packageID]struct{}),
ctx: ctx,
fset: f.FileSet(),
topLevelPkgID: f.meta.id,
}
-
// Start prefetching direct imports.
- for importPath := range f.meta.children {
- go imp.Import(string(importPath))
+ for importID := range f.meta.children {
+ go imp.getPkg(importID)
}
// Type-check package.
- pkg, err := imp.getPkg(f.meta.pkgPath)
+ pkg, err := imp.getPkg(f.meta.id)
if err != nil {
return nil, err
}
@@ -137,24 +134,26 @@
}
func (v *view) link(ctx context.Context, pkgPath packagePath, pkg *packages.Package, parent *metadata) *metadata {
- m, ok := v.mcache.packages[pkgPath]
+ id := packageID(pkg.ID)
+ m, ok := v.mcache.packages[id]
// If a file was added or deleted we need to invalidate the package cache
// so relevant packages get parsed and type checked again.
if ok && !filenamesIdentical(m.files, pkg.CompiledGoFiles) {
- v.invalidatePackage(pkgPath)
+ v.invalidatePackage(id)
}
-
+ // If we haven't seen this package before.
if !ok {
m = &metadata{
pkgPath: pkgPath,
- id: packageID(pkg.ID),
+ id: id,
typesSizes: pkg.TypesSizes,
- parents: make(map[packagePath]bool),
- children: make(map[packagePath]bool),
+ parents: make(map[packageID]bool),
+ children: make(map[packageID]bool),
missingImports: make(map[packagePath]struct{}),
}
- v.mcache.packages[pkgPath] = m
+ v.mcache.packages[id] = m
+ v.mcache.ids[pkgPath] = id
}
// Reset any field that could have changed across calls to packages.Load.
m.name = pkg.Name
@@ -170,26 +169,29 @@
}
// Connect the import graph.
if parent != nil {
- m.parents[parent.pkgPath] = true
- parent.children[pkgPath] = true
+ m.parents[parent.id] = true
+ parent.children[id] = true
}
for importPath, importPkg := range pkg.Imports {
if len(importPkg.Errors) > 0 {
m.missingImports[pkgPath] = struct{}{}
}
- importPkgPath := packagePath(importPath)
- if _, ok := m.children[importPkgPath]; !ok {
- v.link(ctx, importPkgPath, importPkg, m)
+ if _, ok := m.children[packageID(importPkg.ID)]; !ok {
+ v.link(ctx, packagePath(importPath), importPkg, m)
}
}
// Clear out any imports that have been removed.
- for importPath := range m.children {
- if _, ok := pkg.Imports[string(importPath)]; !ok {
- delete(m.children, importPath)
- if child, ok := v.mcache.packages[importPath]; ok {
- delete(child.parents, pkgPath)
- }
+ for importID := range m.children {
+ child, ok := v.mcache.packages[importID]
+ if !ok {
+ continue
}
+ importPath := string(child.pkgPath)
+ if _, ok := pkg.Imports[importPath]; ok {
+ continue
+ }
+ delete(m.children, importID)
+ delete(child.parents, id)
}
return m
}
diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go
index 7a84785..b50011b 100644
--- a/internal/lsp/cache/pkg.go
+++ b/internal/lsp/cache/pkg.go
@@ -137,6 +137,10 @@
return e.Action, nil
}
+func (pkg *pkg) ID() string {
+ return string(pkg.id)
+}
+
func (pkg *pkg) PkgPath() string {
return string(pkg.pkgPath)
}
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 5fc4031..8d6819a 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -81,10 +81,11 @@
filesByURI: make(map[span.URI]viewFile),
filesByBase: make(map[string][]viewFile),
mcache: &metadataCache{
- packages: make(map[packagePath]*metadata),
+ packages: make(map[packageID]*metadata),
+ ids: make(map[packagePath]packageID),
},
pcache: &packageCache{
- packages: make(map[packagePath]*entry),
+ packages: make(map[packageID]*entry),
},
ignoredURIs: make(map[span.URI]struct{}),
}
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index c7ef828..53a81e7 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -70,8 +70,9 @@
}
type metadataCache struct {
- mu sync.Mutex
- packages map[packagePath]*metadata
+ mu sync.Mutex // guards both maps
+ packages map[packageID]*metadata
+ ids map[packagePath]packageID
}
type metadata struct {
@@ -80,7 +81,7 @@
name string
files []string
typesSizes types.Sizes
- parents, children map[packagePath]bool
+ parents, children map[packageID]bool
// missingImports is the set of unresolved imports for this package.
// It contains any packages with `go list` errors.
@@ -89,7 +90,7 @@
type packageCache struct {
mu sync.Mutex
- packages map[packagePath]*entry
+ packages map[packageID]*entry
}
type entry struct {
@@ -247,32 +248,33 @@
// Remove the package and all of its reverse dependencies from the cache.
if f.pkg != nil {
- f.view.remove(f.pkg.pkgPath, map[packagePath]struct{}{})
+ f.view.remove(f.pkg.id, map[packageID]struct{}{})
}
}
// invalidatePackage removes the specified package and dependents from the
// package cache.
-func (v *view) invalidatePackage(pkgPath packagePath) {
+func (v *view) invalidatePackage(id packageID) {
v.pcache.mu.Lock()
defer v.pcache.mu.Unlock()
- v.remove(pkgPath, make(map[packagePath]struct{}))
+
+ v.remove(id, make(map[packageID]struct{}))
}
// remove invalidates a package and its reverse dependencies in the view's
// package cache. It is assumed that the caller has locked both the mutexes
// of both the mcache and the pcache.
-func (v *view) remove(pkgPath packagePath, seen map[packagePath]struct{}) {
- if _, ok := seen[pkgPath]; ok {
+func (v *view) remove(id packageID, seen map[packageID]struct{}) {
+ if _, ok := seen[id]; ok {
return
}
- m, ok := v.mcache.packages[pkgPath]
+ m, ok := v.mcache.packages[id]
if !ok {
return
}
- seen[pkgPath] = struct{}{}
- for parentPkgPath := range m.parents {
- v.remove(parentPkgPath, seen)
+ seen[id] = struct{}{}
+ for parentID := range m.parents {
+ v.remove(parentID, seen)
}
// All of the files in the package may also be holding a pointer to the
// invalidated package.
@@ -283,7 +285,7 @@
}
}
}
- delete(v.pcache.packages, pkgPath)
+ delete(v.pcache.packages, id)
}
// FindFile returns the file if the given URI is already a part of the view.
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index a88bc47..7447cba 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -220,6 +220,7 @@
// Package represents a Go package that has been type-checked. It maintains
// only the relevant fields of a *go/packages.Package.
type Package interface {
+ ID() string
PkgPath() string
GetFilenames() []string
GetSyntax() []*ast.File
