From 8a7d91b684c76b5c59fc578a885867abc58e5948 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Sun, 24 Nov 2024 14:02:52 +0000
Subject: [PATCH] replace listedPackage.Deps in favor of a derived map
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In looking at the cpu and memory profiles, it surfaced that we spent
a lot of time in garbage collection, and a significant amount of the
garbage was produced by gob decoding string slices.

listedPackage.Deps is a list of a package's transitive dependencies,
so as a Go build gets larger, the list also gets larger and larger.

Given that Imports is the list of direct dependencies,
we can reconstruct it ourselves as needed, which is not always.
Moreover, since we want to do lookups, we can build a map directly.

This doesn't directly result in a wall time speed-up,
but it does result in a significant reduction in allocations.
The gob files we store in the disk cache should also be a bit smaller.

            │      old      │               new               │
            │ cached-sec/op │ cached-sec/op  vs base          │
    Build-8     339.5m ± 2%     340.3m ± 1%  ~ (p=0.218 n=10)

            │     old     │                new                 │
            │ mallocs/op  │ mallocs/op   vs base               │
    Build-8   38.08M ± 0%   35.73M ± 0%  -6.18% (p=0.000 n=10)
---
 shared.go | 45 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/shared.go b/shared.go
index 5b1bc31..00bf9da 100644
--- a/shared.go
+++ b/shared.go
@@ -146,7 +146,6 @@ type listedPackage struct {
 	ForTest    string
 	Export     string
 	BuildID    string
-	Deps       []string
 	ImportMap  map[string]string
 	Standard   bool
 
@@ -161,6 +160,11 @@ type listedPackage struct {
 	// between garble processes. Use "Garble" as a prefix to ensure no
 	// collisions with the JSON fields from 'go list'.
 
+	// allDeps is like the Deps field given by 'go list', but in the form of a map
+	// for the sake of fast lookups. It's also unnecessary to consume or store Deps
+	// as returned by 'go list', as it can be reconstructed from Imports.
+	allDeps map[string]struct{}
+
 	// GarbleActionID is a hash combining the Action ID from BuildID,
 	// with Garble's own inputs as per addGarbleToHash.
 	// It is set even when ToObfuscate is false, as it is also used for random
@@ -172,6 +176,33 @@ type listedPackage struct {
 	ToObfuscate bool `json:"-"`
 }
 
+func (p *listedPackage) hasDep(path string) bool {
+	if p.allDeps == nil {
+		p.allDeps = make(map[string]struct{}, len(p.Imports)*2)
+		p.addImportsFrom(p)
+	}
+	_, ok := p.allDeps[path]
+	return ok
+}
+
+func (p *listedPackage) addImportsFrom(from *listedPackage) {
+	for _, path := range from.Imports {
+		if path == "C" {
+			// `go list -json` shows "C" in Imports but not Deps.
+			// See https://go.dev/issue/60453.
+			continue
+		}
+		if path2 := from.ImportMap[path]; path2 != "" {
+			path = path2
+		}
+		if _, ok := p.allDeps[path]; ok {
+			continue // already added
+		}
+		p.allDeps[path] = struct{}{}
+		p.addImportsFrom(sharedCache.ListedPackages[path])
+	}
+}
+
 type packageError struct {
 	Pos string
 	Err string
@@ -419,10 +450,8 @@ func listPackage(from *listedPackage, path string) (*listedPackage, error) {
 
 	// Packages outside std can list any package,
 	// as long as they depend on it directly or indirectly.
-	for _, dep := range from.Deps {
-		if dep == pkg.ImportPath {
-			return pkg, nil
-		}
+	if from.hasDep(pkg.ImportPath) {
+		return pkg, nil
 	}
 
 	// As a special case, any package can list runtime or its dependencies,
@@ -432,10 +461,8 @@ func listPackage(from *listedPackage, path string) (*listedPackage, error) {
 	if pkg.ImportPath == "runtime" {
 		return pkg, nil
 	}
-	for _, dep := range sharedCache.ListedPackages["runtime"].Deps {
-		if dep == pkg.ImportPath {
-			return pkg, nil
-		}
+	if sharedCache.ListedPackages["runtime"].hasDep(pkg.ImportPath) {
+		return pkg, nil
 	}
 
 	return nil, fmt.Errorf("list %s: %w", path, ErrNotDependency)