gopls/internal/cache/analysis: lazily resolve the import map

In CL 613715, we made import lookup lazy to mitigate the quadratic
construction of the import map during type checking. This CL makes the
equivalent change for the analysis driver.

Since analysis does not have fine-grained invalidation, it is even more
susceptible to the performance cost of this quadratic operation:

DiagnosePackageFiles-64
│  before.txt   │              after.txt              │
│    sec/op     │    sec/op     vs base               │
  1691.6m ± ∞ ¹   693.6m ± ∞ ¹  -59.00% (p=0.008 n=5)

│   before.txt   │               after.txt               │
│ cpu_seconds/op │ cpu_seconds/op  vs base               │
     6.480 ± ∞ ¹      3.260 ± ∞ ¹  -49.69% (p=0.008 n=5)

For golang/go#53275

Change-Id: I8690bc85848fe1e36391d4622aa2e3d3f9878f57
Reviewed-on: https://go-review.googlesource.com/c/tools/+/619095
Auto-Submit: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>

gopls/internal/cache/analysis.go

@@ -233,6 +233,9 @@ func (s *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]*metadata.Pac
 	// File set for this batch (entire graph) of analysis.
 	fset := token.NewFileSet()
 
+	// Get the metadata graph once for lock-free access during analysis.
+	meta := s.MetadataGraph()
+
 	// Starting from the root packages and following DepsByPkgPath,
 	// build the DAG of packages we're going to analyze.
 	//
@@ -246,22 +249,23 @@ func (s *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]*metadata.Pac
 	makeNode = func(from *analysisNode, id PackageID) (*analysisNode, error) {
 		an, ok := nodes[id]
 		if !ok {
-			mp := s.Metadata(id)
+			mp := meta.Metadata(id)
 			if mp == nil {
 				return nil, bug.Errorf("no metadata for %s", id)
 			}
 
 			// -- preorder --
 
 			an = &analysisNode{
-				fset:        fset,
-				fsource:     struct{ file.Source }{s}, // expose only ReadFile
-				viewType:    s.View().Type(),
-				mp:          mp,
-				analyzers:   facty, // all nodes run at least the facty analyzers
-				allDeps:     make(map[PackagePath]*analysisNode),
-				exportDeps:  make(map[PackagePath]*analysisNode),
-				stableNames: stableNames,
+				allNodes:     nodes,
+				fset:         fset,
+				fsource:      struct{ file.Source }{s}, // expose only ReadFile
+				viewType:     s.View().Type(),
+				mp:           mp,
+				analyzers:    facty, // all nodes run at least the facty analyzers
+				importLookup: importLookup(mp, meta),
+				exportDeps:   make(map[PackagePath]*analysisNode),
+				stableNames:  stableNames,
 			}
 			nodes[id] = an
 
@@ -275,18 +279,10 @@ func (s *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]*metadata.Pac
 					return nil, err
 				}
 				an.succs[depID] = dep
-
-				// Compute the union of all dependencies.
-				// (This step has quadratic complexity.)
-				for pkgPath, node := range dep.allDeps {
-					an.allDeps[pkgPath] = node
-				}
 			}
 
 			// -- postorder --
 
-			an.allDeps[mp.PkgPath] = an // add self entry (reflexive transitive closure)
-
 			// Add leaf nodes (no successors) directly to queue.
 			if len(an.succs) == 0 {
 				leaves = append(leaves, an)
@@ -520,6 +516,7 @@ func (an *analysisNode) decrefPreds() {
 // its summary field is populated, either from the cache (hit), or by
 // type-checking and analyzing syntax (miss).
 type analysisNode struct {
+	allNodes        map[PackageID]*analysisNode // all nodes, for lazy lookup of transitive dependencies
 	fset            *token.FileSet              // file set shared by entire batch (DAG)
 	fsource         file.Source                 // Snapshot.ReadFile, for use by Pass.ReadFile
 	viewType        ViewType                    // type of view
@@ -530,8 +527,8 @@ type analysisNode struct {
 	succs           map[PackageID]*analysisNode //   (preds -> self -> succs)
 	unfinishedSuccs atomic.Int32
 	unfinishedPreds atomic.Int32                  // effectively a summary.Actions refcount
-	allDeps         map[PackagePath]*analysisNode // all dependencies including self
-	exportDeps      map[PackagePath]*analysisNode // subset of allDeps ref'd by export data (+self)
+	importLookup    func(PackagePath) PackageID   // lazy lookup of path->id
+	exportDeps      map[PackagePath]*analysisNode // nodes referenced by export data (+self)
 	summary         *analyzeSummary               // serializable result of analyzing this package
 	stableNames     map[*analysis.Analyzer]string // cross-process stable names for Analyzers
 
@@ -563,7 +560,9 @@ func (an *analysisNode) _import() (*types.Package, error) {
 			var g errgroup.Group
 			for i, item := range items {
 				path := PackagePath(item.Path)
-				dep, ok := an.allDeps[path]
+
+				id := an.importLookup(path) // possibly ""
+				dep, ok := an.allNodes[id]
 				if !ok {
 					// This early return bypasses Wait; that's ok.
 					return fmt.Errorf("%s: unknown dependency %q", an.mp, path)
@@ -879,9 +878,6 @@ func (an *analysisNode) run(ctx context.Context) (*analyzeSummary, error) {
 		// by export data, it is safe to ignore it.
 		// (In that case dep.types exists but may be unpopulated
 		// or in the process of being populated from export data.)
-		if an.allDeps[PackagePath(path)] == nil {
-			log.Fatalf("fact package %q is not a dependency", path)
-		}
 		return nil
 	})
 
@@ -1094,7 +1090,8 @@ func (an *analysisNode) typeCheck(parsed []*parsego.File) *analysisPackage {
 		log.Fatalf("internal error: bad export data: %v", err)
 	}
 	for _, path := range paths {
-		dep, ok := an.allDeps[path]
+		id := an.importLookup(path) // possibly ""
+		dep, ok := an.allNodes[id]
 		if !ok {
 			log.Fatalf("%s: missing dependency: %q", an, path)
 		}

gopls/internal/cache/check.go

@@ -447,13 +447,22 @@ func storePackageResults(ctx context.Context, ph *packageHandle, p *Package) {
 	}
 }
 
+// Metadata implements the [metadata.Source] interface.
+func (b *typeCheckBatch) Metadata(id PackageID) *metadata.Package {
+	ph := b.getHandle(id)
+	if ph == nil {
+		return nil
+	}
+	return ph.mp
+}
+
 // importPackage loads the given package from its export data in p.exportData
 // (which must already be populated).
 func (b *typeCheckBatch) importPackage(ctx context.Context, mp *metadata.Package, data []byte) (*types.Package, error) {
 	ctx, done := event.Start(ctx, "cache.typeCheckBatch.importPackage", label.Package.Of(string(mp.ID)))
 	defer done()
 
-	importLookup := b.importLookup(mp)
+	importLookup := importLookup(mp, b)
 
 	thisPackage := types.NewPackage(string(mp.PkgPath), string(mp.Name))
 	getPackages := func(items []gcimporter.GetPackagesItem) error {
@@ -592,7 +601,9 @@ func (b *typeCheckBatch) checkPackageForImport(ctx context.Context, ph *packageH
 // package (id).
 //
 // The resulting function is not concurrency safe.
-func (b *typeCheckBatch) importLookup(mp *metadata.Package) func(PackagePath) PackageID {
+func importLookup(mp *metadata.Package, source metadata.Source) func(PackagePath) PackageID {
+	assert(mp != nil, "nil metadata")
+
 	// This function implements an incremental depth first scan through the
 	// package imports. Previous implementations of import mapping built the
 	// entire PackagePath->PackageID mapping eagerly, but that resulted in a
@@ -603,6 +614,7 @@ func (b *typeCheckBatch) importLookup(mp *metadata.Package) func(PackagePath) Pa
 	impMap := map[PackagePath]PackageID{
 		mp.PkgPath: mp.ID,
 	}
+
 	// pending is a FIFO queue of package metadata that has yet to have its
 	// dependencies fully scanned.
 	// Invariant: all entries in pending are already mapped in impMap.
@@ -623,9 +635,11 @@ func (b *typeCheckBatch) importLookup(mp *metadata.Package) func(PackagePath) Pa
 				continue
 			}
 			impMap[depPath] = depID
-			// TODO(rfindley): express this as an operation on the import graph
-			// itself, rather than the set of package handles.
-			pending = append(pending, b.getHandle(depID).mp)
+
+			dep := source.Metadata(depID)
+			assert(dep != nil, "missing dep metadata")
+
+			pending = append(pending, dep)
 			if depPath == pkgPath {
 				// Don't return early; finish processing pkg's deps.
 				id = depID

gopls/internal/cache/metadata/graph.go

@@ -27,6 +27,11 @@ type Graph struct {
 	IDs map[protocol.DocumentURI][]PackageID
 }
 
+// Metadata implements the [Source] interface
+func (g *Graph) Metadata(id PackageID) *Package {
+	return g.Packages[id]
+}
+
 // Update creates a new Graph containing the result of applying the given
 // updates to the receiver, though the receiver is not itself mutated. As a
 // special case, if updates is empty, Update just returns the receiver.

gopls/internal/cache/metadata/metadata.go

@@ -167,9 +167,6 @@ func (mp *Package) IsIntermediateTestVariant() bool {
 }
 
 // A Source maps package IDs to metadata for the packages.
-//
-// TODO(rfindley): replace this with a concrete metadata graph, once it is
-// exposed from the snapshot.
 type Source interface {
 	// Metadata returns the [Package] for the given package ID, or nil if it does
 	// not exist.