internal/lsp/cache: move to a model of caching in a snapshot

This change moves from caching package information the file object to
caching in a map that gets invalidated when content changes.

This simplifies cache invalidation and reduces the number of fields
guarded by the (*goFile).mu lock.

Change-Id: I33fef6e0b18badb97e49052d9d6e3c15047c4b63
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196984
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/cache/gofile.go

@@ -11,6 +11,7 @@ import (
 
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/lsp/telemetry"
+	"golang.org/x/tools/internal/span"
 	errors "golang.org/x/xerrors"
 )
 
@@ -27,9 +28,6 @@ type goFile struct {
 	missingImports map[packagePath]struct{}
 
 	imports []*ast.ImportSpec
-
-	cphs map[packageKey]*checkPackageHandle
-	meta map[packageID]*metadata
 }
 
 type packageKey struct {
@@ -37,117 +35,70 @@ type packageKey struct {
 	mode source.ParseMode
 }
 
-func (f *goFile) CheckPackageHandles(ctx context.Context) ([]source.CheckPackageHandle, error) {
+func (f *goFile) CheckPackageHandles(ctx context.Context) (cphs []source.CheckPackageHandle, err error) {
 	ctx = telemetry.File.With(ctx, f.URI())
 	fh := f.Handle(ctx)
 
-	if f.isDirty(ctx, fh) {
+	cphs = f.isDirty(ctx, fh)
-		if err := f.view.loadParseTypecheck(ctx, f, fh); err != nil {
+	if len(cphs) == 0 {
+		cphs, err = f.view.loadParseTypecheck(ctx, f, fh)
+		if err != nil {
 			return nil, err
 		}
 	}
-
+	if len(cphs) == 0 {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	var results []source.CheckPackageHandle
-	seenIDs := make(map[string]bool)
-	for _, cph := range f.cphs {
-		if seenIDs[cph.ID()] {
-			continue
-		}
-		if cph.mode() < source.ParseFull {
-			continue
-		}
-		results = append(results, cph)
-		seenIDs[cph.ID()] = true
-	}
-	if len(results) == 0 {
 		return nil, errors.Errorf("no CheckPackageHandles for %s", f.URI())
 	}
-	return results, nil
+	return cphs, nil
 }
 
-func (f *goFile) GetActiveReverseDeps(ctx context.Context) (files []source.GoFile) {
+func (v *view) GetActiveReverseDeps(ctx context.Context, uri span.URI) (results []source.CheckPackageHandle) {
-	seen := make(map[packageID]struct{}) // visited packages
+	var (
-	results := make(map[*goFile]struct{})
+		rdeps = v.reverseDependencies(ctx, uri)
-
+		files = v.openFiles(ctx, rdeps)
-	f.view.mu.Lock()
+		seen  = make(map[span.URI]struct{})
-	defer f.view.mu.Unlock()
+	)
-
+	for _, f := range files {
-	f.view.mcache.mu.Lock()
+		if _, ok := seen[f.URI()]; ok {
-	defer f.view.mcache.mu.Unlock()
+			continue
-
-	for _, m := range f.metadata() {
-		f.view.reverseDeps(ctx, seen, results, m.id)
-		for f := range results {
-			if f == nil {
-				continue
-			}
-			// Don't return any of the active files in this package.
-			f.mu.Lock()
-			_, ok := f.meta[m.id]
-			f.mu.Unlock()
-			if ok {
-				continue
-			}
-
-			files = append(files, f)
 		}
-	}
+		gof, ok := f.(source.GoFile)
-	return files
+		if !ok {
-}
+			continue
-
-func (v *view) reverseDeps(ctx context.Context, seen map[packageID]struct{}, results map[*goFile]struct{}, id packageID) {
-	if _, ok := seen[id]; ok {
-		return
-	}
-	seen[id] = struct{}{}
-	m, ok := v.mcache.packages[id]
-	if !ok {
-		return
-	}
-	for _, uri := range m.files {
-		// Call unlocked version of getFile since we hold the lock on the view.
-		if f, err := v.getFile(ctx, uri, source.Go); err == nil && v.session.IsOpen(uri) {
-			results[f.(*goFile)] = struct{}{}
 		}
+		cphs, err := gof.CheckPackageHandles(ctx)
+		if err != nil {
+			continue
+		}
+		cph := source.WidestCheckPackageHandle(cphs)
+		for _, ph := range cph.Files() {
+			seen[ph.File().Identity().URI] = struct{}{}
+		}
+		results = append(results, cph)
 	}
-	for parentID := range m.parents {
+	return results
-		v.reverseDeps(ctx, seen, results, parentID)
-	}
-}
-
-// metadata assumes that the caller holds the f.mu lock.
-func (f *goFile) metadata() []*metadata {
-	result := make([]*metadata, 0, len(f.meta))
-	for _, m := range f.meta {
-		result = append(result, m)
-	}
-	return result
 }
 
 // isDirty is true if the file needs to be type-checked.
 // It assumes that the file's view's mutex is held by the caller.
-func (f *goFile) isDirty(ctx context.Context, fh source.FileHandle) bool {
+func (f *goFile) isDirty(ctx context.Context, fh source.FileHandle) []source.CheckPackageHandle {
-	f.mu.Lock()
+	meta, cphs := f.view.getSnapshot(f.URI())
-	defer f.mu.Unlock()
+	if len(meta) == 0 {
-
+		return nil
-	if len(f.meta) == 0 || len(f.cphs) == 0 {
-		return true
 	}
-	if len(f.missingImports) > 0 {
+	var results []source.CheckPackageHandle
-		return true
+	for key, cph := range cphs {
-	}
+		// If we're explicitly checking if a file needs to be type-checked,
-	for key, cph := range f.cphs {
+		// we need it to be fully parsed.
 		if key.mode != source.ParseFull {
 			continue
 		}
+		// Check if there is a fully-parsed package to which this file belongs.
 		for _, file := range cph.Files() {
 			if file.File().Identity() == fh.Identity() {
-				return false
+				results = append(results, cph)
 			}
 		}
 	}
-	return true
+	return results
 }

internal/lsp/cache/load.go

@@ -19,62 +19,58 @@ import (
 	errors "golang.org/x/xerrors"
 )
 
-func (view *view) loadParseTypecheck(ctx context.Context, f *goFile, fh source.FileHandle) error {
+func (v *view) loadParseTypecheck(ctx context.Context, f *goFile, fh source.FileHandle) ([]source.CheckPackageHandle, error) {
 	ctx, done := trace.StartSpan(ctx, "cache.view.loadParseTypeCheck", telemetry.URI.Of(f.URI()))
 	defer done()
 
-	meta, err := view.load(ctx, f, fh)
+	meta, err := v.load(ctx, f, fh)
 	if err != nil {
-		return err
+		return nil, err
 	}
+	var (
+		cphs    []*checkPackageHandle
+		results []source.CheckPackageHandle
+	)
 	for _, m := range meta {
 		imp := &importer{
-			view:              view,
+			view:              v,
-			config:            view.Config(ctx),
+			config:            v.Config(ctx),
 			seen:              make(map[packageID]struct{}),
 			topLevelPackageID: m.id,
 		}
 		cph, err := imp.checkPackageHandle(ctx, m)
 		if err != nil {
-			log.Error(ctx, "loadParseTypeCheck: failed to get CheckPackageHandle", err, telemetry.Package.Of(m.id))
+			return nil, err
-			continue
 		}
-		// Cache the package type information for the top-level package.
 		for _, ph := range cph.files {
-			file, _, _, err := ph.Parse(ctx)
+			if err := v.cachePerFile(ctx, ph); err != nil {
-			if err != nil {
+				return nil, err
-				return err
-			}
-			f, err := imp.view.GetFile(ctx, ph.File().Identity().URI)
-			if err != nil {
-				return errors.Errorf("no such file %s: %v", ph.File().Identity().URI, err)
-			}
-			gof, ok := f.(*goFile)
-			if !ok {
-				return errors.Errorf("%s is not a Go file", ph.File().Identity().URI)
-			}
-			if err := cachePerFile(ctx, gof, ph.Mode(), file.Imports, cph); err != nil {
-				return err
 			}
 		}
+		cphs = append(cphs, cph)
+		results = append(results, cph)
 	}
-	return nil
+	// Cache the package type information for the top-level package.
+	v.updatePackages(cphs)
+	return results, nil
 }
 
-func cachePerFile(ctx context.Context, f *goFile, mode source.ParseMode, imports []*ast.ImportSpec, cph *checkPackageHandle) error {
+func (v *view) cachePerFile(ctx context.Context, ph source.ParseGoHandle) error {
-	f.mu.Lock()
+	file, _, _, err := ph.Parse(ctx)
-	defer f.mu.Unlock()
+	if err != nil {
-
+		return err
-	f.imports = imports
-
-	if f.cphs == nil {
-		f.cphs = make(map[packageKey]*checkPackageHandle)
 	}
-	f.cphs[packageKey{
+	f, err := v.GetFile(ctx, ph.File().Identity().URI)
-		id:   cph.m.id,
+	if err != nil {
-		mode: mode,
+		return err
-	}] = cph
+	}
-
+	gof, ok := f.(*goFile)
+	if !ok {
+		return errors.Errorf("%s is not a Go file", ph.File().Identity().URI)
+	}
+	gof.mu.Lock()
+	gof.imports = file.Imports
+	gof.mu.Unlock()
 	return nil
 }
 
@@ -82,12 +78,6 @@ func (view *view) load(ctx context.Context, f *goFile, fh source.FileHandle) ([]
 	ctx, done := trace.StartSpan(ctx, "cache.view.load", telemetry.URI.Of(f.URI()))
 	defer done()
 
-	view.mu.Lock()
-	defer view.mu.Unlock()
-
-	view.mcache.mu.Lock()
-	defer view.mcache.mu.Unlock()
-
 	// Get the metadata for the file.
 	meta, err := view.checkMetadata(ctx, f, fh)
 	if err != nil {
@@ -101,23 +91,24 @@ func (view *view) load(ctx context.Context, f *goFile, fh source.FileHandle) ([]
 
 // checkMetadata determines if we should run go/packages.Load for this file.
 // If yes, update the metadata for the file and its package.
-func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandle) (metadata []*metadata, err error) {
+func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandle) ([]*metadata, error) {
-	// Check if we need to re-run go/packages before loading the package.
+	var shouldRunGopackages bool
-	var runGopackages bool
-	func() {
-		f.mu.Lock()
-		defer f.mu.Unlock()
 
-		runGopackages, err = v.shouldRunGopackages(ctx, f, fh)
+	m := v.getMetadata(fh.Identity().URI)
-		metadata = f.metadata()
+	if len(m) == 0 {
-	}()
+		shouldRunGopackages = true
+	}
+	// Get file content in case we don't already have it.
+	parsed, _, _, err := v.session.cache.ParseGoHandle(fh, source.ParseHeader).Parse(ctx)
 	if err != nil {
 		return nil, err
 	}
+	// Check if we need to re-run go/packages before loading the package.
+	shouldRunGopackages = shouldRunGopackages || v.shouldRunGopackages(ctx, f, parsed, m)
 
 	// The package metadata is correct as-is, so just return it.
-	if !runGopackages {
+	if !shouldRunGopackages {
-		return metadata, nil
+		return m, nil
 	}
 
 	// Don't bother running go/packages if the context has been canceled.
@@ -129,6 +120,8 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandl
 	defer done()
 
 	pkgs, err := packages.Load(v.Config(ctx), fmt.Sprintf("file=%s", f.filename()))
+	log.Print(ctx, "go/packages.Load", tag.Of("packages", len(pkgs)))
+
 	if len(pkgs) == 0 {
 		if err == nil {
 			err = errors.Errorf("go/packages.Load: no packages found for %s", f.filename())
@@ -136,200 +129,32 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandl
 		// Return this error as a diagnostic to the user.
 		return nil, err
 	}
-	// Track missing imports as we look at the package's errors.
+	return v.updateMetadata(ctx, f.URI(), pkgs)
-	missingImports := make(map[packagePath]struct{})
-
-	// Clear metadata since we are re-running go/packages.
-	// Reset the file's metadata and type information if we are re-running `go list`.
-	f.mu.Lock()
-	for k := range f.meta {
-		delete(f.meta, k)
-	}
-	for k := range f.cphs {
-		delete(f.cphs, k)
-	}
-	f.mu.Unlock()
-
-	log.Print(ctx, "go/packages.Load", tag.Of("packages", len(pkgs)))
-	for _, pkg := range pkgs {
-		log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles))
-		// Build the import graph for this package.
-		if err := v.link(ctx, &importGraph{
-			pkgPath:        packagePath(pkg.PkgPath),
-			pkg:            pkg,
-			parent:         nil,
-			missingImports: make(map[packagePath]struct{}),
-		}); err != nil {
-			return nil, err
-		}
-	}
-	m, err := validateMetadata(ctx, missingImports, f)
-	if err != nil {
-		return nil, err
-	}
-	return m, nil
-}
-
-func validateMetadata(ctx context.Context, missingImports map[packagePath]struct{}, f *goFile) ([]*metadata, error) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	// If `go list` failed to get data for the file in question (this should never happen).
-	if len(f.meta) == 0 {
-		return nil, errors.Errorf("loadParseTypecheck: no metadata found for %v", f.filename())
-	}
-
-	// If we have already seen these missing imports before, and we have type information,
-	// there is no need to continue.
-	if sameSet(missingImports, f.missingImports) && len(f.cphs) != 0 {
-		return nil, nil
-	}
-
-	// Otherwise, update the missing imports map.
-	f.missingImports = missingImports
-
-	return f.metadata(), nil
-}
-
-func sameSet(x, y map[packagePath]struct{}) bool {
-	if len(x) != len(y) {
-		return false
-	}
-	for k := range x {
-		if _, ok := y[k]; !ok {
-			return false
-		}
-	}
-	return true
 }
 
 // shouldRunGopackages reparses a file's package and import declarations to
 // determine if they have changed.
 // It assumes that the caller holds the lock on the f.mu lock.
-func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, fh source.FileHandle) (result bool, err error) {
+func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, file *ast.File, metadata []*metadata) bool {
-	if len(f.meta) == 0 || len(f.missingImports) > 0 {
-		return true, nil
-	}
-	// Get file content in case we don't already have it.
-	parsed, _, _, err := v.session.cache.ParseGoHandle(fh, source.ParseHeader).Parse(ctx)
-	if err != nil {
-		return false, err
-	}
 	// Check if the package's name has changed, by checking if this is a filename
 	// we already know about, and if so, check if its package name has changed.
-	for _, m := range f.meta {
+	for _, m := range metadata {
 		for _, uri := range m.files {
 			if span.CompareURI(uri, f.URI()) == 0 {
-				if m.name != parsed.Name.Name {
+				if m.name != file.Name.Name {
-					return true, nil
+					return true
 				}
 			}
 		}
 	}
 	// If the package's imports have changed, re-run `go list`.
-	if len(f.imports) != len(parsed.Imports) {
+	if len(f.imports) != len(file.Imports) {
-		return true, nil
+		return true
 	}
 	for i, importSpec := range f.imports {
-		if importSpec.Path.Value != parsed.Imports[i].Path.Value {
+		if importSpec.Path.Value != file.Imports[i].Path.Value {
-			return true, nil
+			return true
 		}
 	}
-	return false, nil
+	return false
-}
-
-type importGraph struct {
-	pkgPath        packagePath
-	pkg            *packages.Package
-	parent         *metadata
-	missingImports map[packagePath]struct{}
-}
-
-func (v *view) link(ctx context.Context, g *importGraph) error {
-	// Recreate the metadata rather than reusing it to avoid locking.
-	m := &metadata{
-		id:         packageID(g.pkg.ID),
-		pkgPath:    g.pkgPath,
-		name:       g.pkg.Name,
-		typesSizes: g.pkg.TypesSizes,
-		errors:     g.pkg.Errors,
-	}
-	for _, filename := range g.pkg.CompiledGoFiles {
-		m.files = append(m.files, span.FileURI(filename))
-
-		// Call the unlocked version of getFile since we are holding the view's mutex.
-		f, err := v.getFile(ctx, span.FileURI(filename), source.Go)
-		if err != nil {
-			log.Error(ctx, "no file", err, telemetry.File.Of(filename))
-			continue
-		}
-		gof, ok := f.(*goFile)
-		if !ok {
-			log.Error(ctx, "not a Go file", nil, telemetry.File.Of(filename))
-			continue
-		}
-		// Cache the metadata for this file.
-		gof.mu.Lock()
-		if gof.meta == nil {
-			gof.meta = make(map[packageID]*metadata)
-		}
-		gof.meta[m.id] = m
-		gof.mu.Unlock()
-	}
-
-	// Preserve the import graph.
-	if original, ok := v.mcache.packages[m.id]; ok {
-		m.children = original.children
-		m.parents = original.parents
-	}
-	if m.children == nil {
-		m.children = make(map[packageID]*metadata)
-	}
-	if m.parents == nil {
-		m.parents = make(map[packageID]bool)
-	}
-
-	// Add the metadata to the cache.
-	v.mcache.packages[m.id] = m
-
-	// Connect the import graph.
-	if g.parent != nil {
-		m.parents[g.parent.id] = true
-		g.parent.children[m.id] = m
-	}
-	for importPath, importPkg := range g.pkg.Imports {
-		importPkgPath := packagePath(importPath)
-		if importPkgPath == g.pkgPath {
-			return fmt.Errorf("cycle detected in %s", importPath)
-		}
-		// Don't remember any imports with significant errors.
-		if importPkgPath != "unsafe" && len(importPkg.CompiledGoFiles) == 0 {
-			g.missingImports[importPkgPath] = struct{}{}
-			continue
-		}
-		if _, ok := m.children[packageID(importPkg.ID)]; !ok {
-			if err := v.link(ctx, &importGraph{
-				pkgPath:        importPkgPath,
-				pkg:            importPkg,
-				parent:         m,
-				missingImports: g.missingImports,
-			}); err != nil {
-				log.Error(ctx, "error in dependency", err)
-			}
-		}
-	}
-	// Clear out any imports that have been removed since the package was last loaded.
-	for importID := range m.children {
-		child, ok := v.mcache.packages[importID]
-		if !ok {
-			continue
-		}
-		importPath := string(child.pkgPath)
-		if _, ok := g.pkg.Imports[importPath]; ok {
-			continue
-		}
-		delete(m.children, importID)
-		delete(child.parents, m.id)
-	}
-	return nil
 }

internal/lsp/cache/session.go

@@ -98,8 +98,10 @@ func (s *session) NewView(ctx context.Context, name string, folder span.URI, opt
 		folder:        folder,
 		filesByURI:    make(map[span.URI]viewFile),
 		filesByBase:   make(map[string][]viewFile),
-		mcache: &metadataCache{
+		snapshot: &snapshot{
-			packages: make(map[packageID]*metadata),
+			packages: make(map[span.URI]map[packageKey]*checkPackageHandle),
+			ids:      make(map[span.URI][]packageID),
+			metadata: make(map[packageID]*metadata),
 		},
 		ignoredURIs: make(map[span.URI]struct{}),
 		builtin:     &builtinPkg{},
@@ -144,6 +146,19 @@ func (s *session) ViewOf(uri span.URI) source.View {
 	return v
 }
 
+func (s *session) viewsOf(uri span.URI) []*view {
+	s.viewMu.Lock()
+	defer s.viewMu.Unlock()
+
+	var views []*view
+	for _, view := range s.views {
+		if strings.HasPrefix(string(uri), string(view.Folder())) {
+			views = append(views, view)
+		}
+	}
+	return views
+}
+
 func (s *session) Views() []source.View {
 	s.viewMu.Lock()
 	defer s.viewMu.Unlock()
@@ -219,21 +234,7 @@ func (s *session) DidOpen(ctx context.Context, uri span.URI, kind source.FileKin
 	// A file may be in multiple views.
 	for _, view := range s.views {
 		if strings.HasPrefix(string(uri), string(view.Folder())) {
-			f, err := view.GetFile(ctx, uri)
+			view.invalidateMetadata(uri)
-			if err != nil {
-				log.Error(ctx, "error getting file", nil, telemetry.File)
-				return
-			}
-			gof, ok := f.(*goFile)
-			if !ok {
-				log.Error(ctx, "not a Go file", nil, telemetry.File)
-				return
-			}
-			// Force a reload of the package metadata by clearing the cached data.
-			gof.mu.Lock()
-			gof.meta = make(map[packageID]*metadata)
-			gof.cphs = make(map[packageKey]*checkPackageHandle)
-			gof.mu.Unlock()
 		}
 	}
 }
@@ -341,15 +342,16 @@ func (s *session) buildOverlay() map[string][]byte {
 	return overlays
 }
 
-func (s *session) DidChangeOutOfBand(ctx context.Context, f source.GoFile, changeType protocol.FileChangeType) {
+func (s *session) DidChangeOutOfBand(ctx context.Context, uri span.URI, changeType protocol.FileChangeType) {
 	if changeType == protocol.Deleted {
 		// After a deletion we must invalidate the package's metadata to
-		// force a go/packages invocation to refresh the package's file
+		// force a go/packages invocation to refresh the package's file list.
-		// list.
+		views := s.viewsOf(uri)
-		f.(*goFile).invalidateMeta(ctx)
+		for _, v := range views {
+			v.invalidateMetadata(uri)
+		}
 	}
-
+	s.filesWatchMap.Notify(uri)
-	s.filesWatchMap.Notify(f.URI())
 }
 
 func (o *overlay) FileSystem() source.FileSystem {

internal/lsp/cache/snapshot.go

@@ -0,0 +1,309 @@
+package cache
+
+import (
+	"context"
+	"go/types"
+
+	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/internal/span"
+	"golang.org/x/tools/internal/telemetry/log"
+	"golang.org/x/tools/internal/telemetry/tag"
+	errors "golang.org/x/xerrors"
+)
+
+type snapshot struct {
+	id uint64
+
+	packages map[span.URI]map[packageKey]*checkPackageHandle
+	ids      map[span.URI][]packageID
+	metadata map[packageID]*metadata
+}
+
+type metadata struct {
+	id         packageID
+	pkgPath    packagePath
+	name       string
+	files      []span.URI
+	typesSizes types.Sizes
+	parents    map[packageID]bool
+	children   map[packageID]*metadata
+	errors     []packages.Error
+}
+
+func (v *view) getSnapshot(uri span.URI) ([]*metadata, map[packageKey]*checkPackageHandle) {
+	v.snapshotMu.Lock()
+	defer v.snapshotMu.Unlock()
+
+	var m []*metadata
+	for _, id := range v.snapshot.ids[uri] {
+		m = append(m, v.snapshot.metadata[id])
+	}
+	return m, v.snapshot.packages[uri]
+}
+
+func (v *view) getMetadata(uri span.URI) []*metadata {
+	v.snapshotMu.Lock()
+	defer v.snapshotMu.Unlock()
+
+	var m []*metadata
+	for _, id := range v.snapshot.ids[uri] {
+		m = append(m, v.snapshot.metadata[id])
+	}
+	return m
+}
+
+func (v *view) getPackages(uri span.URI) map[packageKey]*checkPackageHandle {
+	v.snapshotMu.Lock()
+	defer v.snapshotMu.Unlock()
+
+	return v.snapshot.packages[uri]
+}
+
+func (v *view) updateMetadata(ctx context.Context, uri span.URI, pkgs []*packages.Package) ([]*metadata, error) {
+	v.snapshotMu.Lock()
+	defer v.snapshotMu.Unlock()
+
+	// Clear metadata since we are re-running go/packages.
+	without := make(map[span.URI]struct{})
+	for _, id := range v.snapshot.ids[uri] {
+		v.remove(id, without, map[packageID]struct{}{})
+	}
+	v.snapshot = v.snapshot.cloneMetadata(without)
+
+	var results []*metadata
+	for _, pkg := range pkgs {
+		log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles))
+
+		// Build the import graph for this package.
+		if err := v.updateImportGraph(ctx, &importGraph{
+			pkgPath: packagePath(pkg.PkgPath),
+			pkg:     pkg,
+			parent:  nil,
+		}); err != nil {
+			return nil, err
+		}
+		results = append(results, v.snapshot.metadata[packageID(pkg.ID)])
+	}
+	return results, nil
+}
+
+type importGraph struct {
+	pkgPath packagePath
+	pkg     *packages.Package
+	parent  *metadata
+}
+
+func (v *view) updateImportGraph(ctx context.Context, g *importGraph) error {
+	// Recreate the metadata rather than reusing it to avoid locking.
+	m := &metadata{
+		id:         packageID(g.pkg.ID),
+		pkgPath:    g.pkgPath,
+		name:       g.pkg.Name,
+		typesSizes: g.pkg.TypesSizes,
+		errors:     g.pkg.Errors,
+	}
+	for _, filename := range g.pkg.CompiledGoFiles {
+		uri := span.FileURI(filename)
+		v.snapshot.ids[uri] = append(v.snapshot.ids[uri], m.id)
+		m.files = append(m.files, uri)
+	}
+	// Preserve the import graph.
+	if original, ok := v.snapshot.metadata[m.id]; ok {
+		m.children = original.children
+		m.parents = original.parents
+	}
+	if m.children == nil {
+		m.children = make(map[packageID]*metadata)
+	}
+	if m.parents == nil {
+		m.parents = make(map[packageID]bool)
+	}
+
+	// Add the metadata to the cache.
+	v.snapshot.metadata[m.id] = m
+
+	// Connect the import graph.
+	if g.parent != nil {
+		m.parents[g.parent.id] = true
+		g.parent.children[m.id] = m
+	}
+	for importPath, importPkg := range g.pkg.Imports {
+		importPkgPath := packagePath(importPath)
+		if importPkgPath == g.pkgPath {
+			return errors.Errorf("cycle detected in %s", importPath)
+		}
+		// Don't remember any imports with significant errors.
+		if importPkgPath != "unsafe" && len(importPkg.CompiledGoFiles) == 0 {
+			continue
+		}
+		if _, ok := m.children[packageID(importPkg.ID)]; !ok {
+			if err := v.updateImportGraph(ctx, &importGraph{
+				pkgPath: importPkgPath,
+				pkg:     importPkg,
+				parent:  m,
+			}); err != nil {
+				log.Error(ctx, "error in dependency", err)
+			}
+		}
+	}
+	// Clear out any imports that have been removed since the package was last loaded.
+	for importID := range m.children {
+		child, ok := v.snapshot.metadata[importID]
+		if !ok {
+			continue
+		}
+		importPath := string(child.pkgPath)
+		if _, ok := g.pkg.Imports[importPath]; ok {
+			continue
+		}
+		delete(m.children, importID)
+		delete(child.parents, m.id)
+	}
+	return nil
+}
+
+func (v *view) updatePackages(cphs []*checkPackageHandle) {
+	v.snapshotMu.Lock()
+	defer v.snapshotMu.Unlock()
+
+	for _, cph := range cphs {
+		for _, ph := range cph.files {
+			uri := ph.File().Identity().URI
+			if _, ok := v.snapshot.packages[uri]; !ok {
+				v.snapshot.packages[uri] = make(map[packageKey]*checkPackageHandle)
+			}
+			v.snapshot.packages[uri][packageKey{
+				id:   cph.m.id,
+				mode: ph.Mode(),
+			}] = cph
+		}
+	}
+}
+
+// invalidateContent invalidates the content of a Go file,
+// including any position and type information that depends on it.
+func (v *view) invalidateContent(ctx context.Context, f *goFile) {
+	f.handleMu.Lock()
+	defer f.handleMu.Unlock()
+
+	without := make(map[span.URI]struct{})
+
+	// Remove the package and all of its reverse dependencies from the cache.
+	v.snapshotMu.Lock()
+	defer v.snapshotMu.Unlock()
+
+	for _, id := range v.snapshot.ids[f.URI()] {
+		f.view.remove(id, without, map[packageID]struct{}{})
+	}
+	v.snapshot = v.snapshot.clonePackages(without)
+	f.handle = nil
+}
+
+// invalidateMeta invalidates package metadata for all files in f's
+// package. This forces f's package's metadata to be reloaded next
+// time the package is checked.
+func (v *view) invalidateMetadata(uri span.URI) {
+	v.snapshotMu.Lock()
+	defer v.snapshotMu.Unlock()
+
+	without := make(map[span.URI]struct{})
+
+	for _, id := range v.snapshot.ids[uri] {
+		v.remove(id, without, map[packageID]struct{}{})
+	}
+	v.snapshot = v.snapshot.cloneMetadata(without)
+}
+
+// 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(id packageID, toDelete map[span.URI]struct{}, seen map[packageID]struct{}) {
+	if _, ok := seen[id]; ok {
+		return
+	}
+	m, ok := v.snapshot.metadata[id]
+	if !ok {
+		return
+	}
+	seen[id] = struct{}{}
+	for parentID := range m.parents {
+		v.remove(parentID, toDelete, seen)
+	}
+	for _, uri := range m.files {
+		toDelete[uri] = struct{}{}
+	}
+}
+
+func (s *snapshot) clonePackages(without map[span.URI]struct{}) *snapshot {
+	result := &snapshot{
+		id:       s.id + 1,
+		packages: make(map[span.URI]map[packageKey]*checkPackageHandle),
+		ids:      s.ids,
+		metadata: s.metadata,
+	}
+	for k, v := range s.packages {
+		if _, ok := without[k]; ok {
+			continue
+		}
+		result.packages[k] = v
+	}
+	return result
+}
+
+func (s *snapshot) cloneMetadata(without map[span.URI]struct{}) *snapshot {
+	result := &snapshot{
+		id:       s.id + 1,
+		packages: s.packages,
+		ids:      make(map[span.URI][]packageID),
+		metadata: make(map[packageID]*metadata),
+	}
+	withoutIDs := make(map[packageID]struct{})
+	for k, ids := range s.ids {
+		if _, ok := without[k]; ok {
+			for _, id := range ids {
+				withoutIDs[id] = struct{}{}
+			}
+			continue
+		}
+		result.ids[k] = ids
+	}
+	for k, v := range s.metadata {
+		if _, ok := withoutIDs[k]; ok {
+			continue
+		}
+		result.metadata[k] = v
+	}
+	return result
+}
+
+func (v *view) reverseDependencies(ctx context.Context, uri span.URI) map[span.URI]struct{} {
+	seen := make(map[packageID]struct{})
+	uris := make(map[span.URI]struct{})
+
+	v.snapshotMu.Lock()
+	defer v.snapshotMu.Unlock()
+
+	for _, id := range v.snapshot.ids[uri] {
+		v.rdeps(id, seen, uris, id)
+	}
+	return uris
+}
+
+func (v *view) rdeps(topID packageID, seen map[packageID]struct{}, results map[span.URI]struct{}, id packageID) {
+	if _, ok := seen[id]; ok {
+		return
+	}
+	seen[id] = struct{}{}
+	m, ok := v.snapshot.metadata[id]
+	if !ok {
+		return
+	}
+	if id != topID {
+		for _, uri := range m.files {
+			results[uri] = struct{}{}
+		}
+	}
+	for parentID := range m.parents {
+		v.rdeps(topID, seen, results, parentID)
+	}
+}

internal/lsp/cache/view.go

@@ -10,7 +10,6 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
-	"go/types"
 	"os"
 	"os/exec"
 	"strings"
@@ -21,7 +20,6 @@ import (
 	"golang.org/x/tools/internal/imports"
 	"golang.org/x/tools/internal/lsp/debug"
 	"golang.org/x/tools/internal/lsp/source"
-	"golang.org/x/tools/internal/lsp/telemetry"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/log"
 	errors "golang.org/x/xerrors"
@@ -72,8 +70,8 @@ type view struct {
 	filesByURI  map[span.URI]viewFile
 	filesByBase map[string][]viewFile
 
-	// mcache caches metadata for the packages of the opened files in a view.
+	snapshotMu sync.Mutex
-	mcache *metadataCache
+	snapshot   *snapshot
 
 	// builtin is used to resolve builtin types.
 	builtin *builtinPkg
@@ -85,23 +83,6 @@ type view struct {
 	analyzers []*analysis.Analyzer
 }
 
-type metadataCache struct {
-	mu       sync.Mutex // guards both maps
-	packages map[packageID]*metadata
-}
-
-type metadata struct {
-	id         packageID
-	pkgPath    packagePath
-	name       string
-	files      []span.URI
-	key        string
-	typesSizes types.Sizes
-	parents    map[packageID]bool
-	children   map[packageID]*metadata
-	errors     []packages.Error
-}
-
 func (v *view) Session() source.Session {
 	return v.session
 }
@@ -324,102 +305,6 @@ func (v *view) SetContent(ctx context.Context, uri span.URI, content []byte) (bo
 	return false, nil
 }
 
-// invalidateContent invalidates the content of a Go file,
-// including any position and type information that depends on it.
-func (f *goFile) invalidateContent(ctx context.Context) {
-	// Mutex acquisition order here is important. It must match the order
-	// in loadParseTypecheck to avoid deadlocks.
-	f.view.mcache.mu.Lock()
-	defer f.view.mcache.mu.Unlock()
-
-	toDelete := make(map[packageID]bool)
-	f.mu.Lock()
-	for key, cph := range f.cphs {
-		if cph != nil {
-			toDelete[key.id] = true
-		}
-	}
-	f.mu.Unlock()
-
-	f.handleMu.Lock()
-	defer f.handleMu.Unlock()
-
-	// Remove the package and all of its reverse dependencies from the cache.
-	for id := range toDelete {
-		f.view.remove(ctx, id, map[packageID]struct{}{})
-	}
-
-	f.handle = nil
-}
-
-// invalidateMeta invalidates package metadata for all files in f's
-// package. This forces f's package's metadata to be reloaded next
-// time the package is checked.
-func (f *goFile) invalidateMeta(ctx context.Context) {
-	cphs, err := f.CheckPackageHandles(ctx)
-	if err != nil {
-		log.Error(ctx, "invalidateMeta: GetPackages", err, telemetry.File.Of(f.URI()))
-		return
-	}
-
-	for _, pkg := range cphs {
-		for _, pgh := range pkg.Files() {
-			uri := pgh.File().Identity().URI
-			if gof, _ := f.view.FindFile(ctx, uri).(*goFile); gof != nil {
-				gof.mu.Lock()
-				gof.meta = nil
-				gof.mu.Unlock()
-			}
-		}
-		f.view.mcache.mu.Lock()
-		delete(f.view.mcache.packages, packageID(pkg.ID()))
-		f.view.mcache.mu.Unlock()
-	}
-}
-
-// 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(ctx context.Context, id packageID, seen map[packageID]struct{}) {
-	if _, ok := seen[id]; ok {
-		return
-	}
-	m, ok := v.mcache.packages[id]
-	if !ok {
-		return
-	}
-	seen[id] = struct{}{}
-	for parentID := range m.parents {
-		v.remove(ctx, parentID, seen)
-	}
-	// All of the files in the package may also be holding a pointer to the
-	// invalidated package.
-	for _, uri := range m.files {
-		f, err := v.findFile(uri)
-		if err != nil {
-			log.Error(ctx, "cannot find file", err, telemetry.File.Of(f.URI()))
-			continue
-		}
-		gof, ok := f.(*goFile)
-		if !ok {
-			log.Error(ctx, "non-Go file", nil, telemetry.File.Of(f.URI()))
-			continue
-		}
-		gof.mu.Lock()
-		for _, mode := range []source.ParseMode{
-			source.ParseExported,
-			source.ParseFull,
-		} {
-			delete(gof.cphs, packageKey{
-				id:   id,
-				mode: mode,
-			})
-		}
-		gof.mu.Unlock()
-	}
-	return
-}
-
 // FindFile returns the file if the given URI is already a part of the view.
 func (v *view) FindFile(ctx context.Context, uri span.URI) source.File {
 	v.mu.Lock()
@@ -481,7 +366,7 @@ func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind)
 			if !ok {
 				return
 			}
-			gof.invalidateContent(ctx)
+			v.invalidateContent(ctx, gof)
 		})
 	}
 	v.mapFile(uri, f)
@@ -540,6 +425,19 @@ func (v *view) mapFile(uri span.URI, f viewFile) {
 	}
 }
 
+func (v *view) openFiles(ctx context.Context, uris map[span.URI]struct{}) (results []source.File) {
+	v.mu.Lock()
+	defer v.mu.Unlock()
+
+	for uri := range uris {
+		// Call unlocked version of getFile since we hold the lock on the view.
+		if f, err := v.getFile(ctx, uri, source.Go); err == nil && v.session.IsOpen(uri) {
+			results = append(results, f)
+		}
+	}
+	return results
+}
+
 type debugView struct{ *view }
 
 func (v debugView) ID() string             { return v.id }

internal/lsp/source/diagnostics.go

@@ -84,13 +84,8 @@ func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[
 		}
 	}
 	// Updates to the diagnostics for this package may need to be propagated.
-	revDeps := f.GetActiveReverseDeps(ctx)
+	revDeps := view.GetActiveReverseDeps(ctx, f.URI())
-	for _, f := range revDeps {
+	for _, cph := range revDeps {
-		cphs, err := f.CheckPackageHandles(ctx)
-		if err != nil {
-			return nil, "", err
-		}
-		cph := WidestCheckPackageHandle(cphs)
 		pkg, err := cph.Check(ctx)
 		if err != nil {
 			return nil, warningMsg, err

internal/lsp/source/view.go

@@ -192,7 +192,7 @@ type Session interface {
 
 	// DidChangeOutOfBand is called when a file under the root folder
 	// changes. The file is not necessarily open in the editor.
-	DidChangeOutOfBand(ctx context.Context, f GoFile, change protocol.FileChangeType)
+	DidChangeOutOfBand(ctx context.Context, uri span.URI, change protocol.FileChangeType)
 
 	// Options returns a copy of the SessionOptions for this session.
 	Options() Options
@@ -254,6 +254,10 @@ type View interface {
 
 	// Analyzers returns the set of Analyzers active for this view.
 	Analyzers() []*analysis.Analyzer
+
+	// GetActiveReverseDeps returns the active files belonging to the reverse
+	// dependencies of this file's package.
+	GetActiveReverseDeps(ctx context.Context, uri span.URI) []CheckPackageHandle
 }
 
 // File represents a source file of any type.
@@ -270,10 +274,6 @@ type GoFile interface {
 	// GetCheckPackageHandles returns the CheckPackageHandles for the packages
 	// that this file belongs to.
 	CheckPackageHandles(ctx context.Context) ([]CheckPackageHandle, error)
-
-	// GetActiveReverseDeps returns the active files belonging to the reverse
-	// dependencies of this file's package.
-	GetActiveReverseDeps(ctx context.Context) []GoFile
 }
 
 type ModFile interface {

internal/lsp/watched_files.go

@@ -44,7 +44,7 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
 			case protocol.Changed:
 				log.Print(ctx, "watched file changed", telemetry.File)
 
-				s.session.DidChangeOutOfBand(ctx, gof, change.Type)
+				s.session.DidChangeOutOfBand(ctx, uri, change.Type)
 
 				// Refresh diagnostics to reflect updated file contents.
 				go s.diagnostics(view, uri)
@@ -77,7 +77,7 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
 						break
 					}
 				}
-				s.session.DidChangeOutOfBand(ctx, gof, change.Type)
+				s.session.DidChangeOutOfBand(ctx, uri, change.Type)
 
 				// If this was the only file in the package, clear its diagnostics.
 				if otherFile == nil {