[WIP] Faster query changes

src/core/graph.go

@@ -153,6 +153,18 @@ func (graph *BuildGraph) AllTargets() BuildTargets {
 	return targets
 }
 
+// IterTargets returns a sequence of all targets in the graph.
+// TODO(peterebden): Make this AllTargets() (for consistency) and bin the other version (being careful about shell_output.go and xml_results.go which might care about order)
+func (graph *BuildGraph) IterTargets() iter.Seq[*BuildTarget] {
+	return func(yield func(*BuildTarget) bool) {
+		for _, v := range graph.targets.Items() {
+			if !yield(v) {
+				break
+			}
+		}
+	}
+}
+
 // AllPackages returns an iterator over all packages in the graph. They are returned in no particular order.
 func (graph *BuildGraph) AllPackages() iter.Seq[*Package] {
 	return func(yield func(*Package) bool) {

src/please.go

@@ -998,6 +998,11 @@ var buildFunctions = map[string]func() int{
 		}
 		readConfig()
 		_, before := runBuild(core.WholeGraph, buildOpts{})
+		log.Debug("Summarising before state...")
+		summary := query.Summarise(before)
+		before = nil
+		go runtime.GC()
+		log.Debug("Parsing after state...")
 		// N.B. Ignore failure here; if we can't parse the graph before then it will suffice to
 		//      assume that anything we don't know about has changed.
 		if err := scm.Checkout(original); err != nil {
@@ -1008,7 +1013,7 @@ var buildFunctions = map[string]func() int{
 		if !success {
 			return 1
 		}
-		for _, target := range query.DiffGraphs(before, after, files, level, includeSubrepos) {
+		for _, target := range query.DiffGraphs(summary, after, files, level, includeSubrepos) {
 			fmt.Println(target.String())
 		}
 		return 0

src/query/BUILD

@@ -1,3 +1,5 @@
+subinclude("//build_defs:benchmark")
+
 go_library(
     name = "query",
     srcs = glob(
@@ -20,7 +22,10 @@ go_library(
 
 go_test(
     name = "query_test",
-    srcs = glob(["*_test.go"]),
+    srcs = glob(
+        ["*_test.go"],
+        exclude = ["*_benchmark_test.go"],
+    ),
     data = ["completions_test_repo"],
     deps = [
         ":query",
@@ -31,3 +36,13 @@ go_test(
         "//src/parse",
     ],
 )
+
+benchmark(
+    name = "changes_benchmark",
+    srcs = ["changes_benchmark_test.go"],
+    deps = [
+        ":query",
+        "///third_party/go/gopkg.in_op_go-logging.v1//:go-logging.v1",
+        "//src/core",
+    ],
+)

src/query/changes.go

@@ -4,7 +4,10 @@ import (
 	"bytes"
 	"crypto/sha1"
 	"path/filepath"
+	"runtime"
+	"slices"
 	"sort"
+	"sync"
 
 	"github.com/thought-machine/please/src/build"
 	"github.com/thought-machine/please/src/core"
@@ -13,7 +16,7 @@ import (
 // DiffGraphs calculates the difference between two build graphs.
 // Note that this is not symmetric; targets that have been removed from 'before' do not appear
 // (because this is designed to be fed into 'plz test' and we can't test targets that no longer exist).
-func DiffGraphs(before, after *core.BuildState, files []string, level int, includeSubrepos bool) core.BuildLabels {
+func DiffGraphs(before *GraphSummary, after *core.BuildState, files []string, level int, includeSubrepos bool) core.BuildLabels {
 	log.Notice("Calculating difference...")
 	changed := diffGraphs(before, after)
 	log.Debugf("Number of changed targets on a non-recursive diff between before and after build graphs: %d", len(changed))
@@ -29,14 +32,36 @@ func Changes(state *core.BuildState, files []string, level int, includeSubrepos
 }
 
 // diffGraphs performs a non-recursive diff of two build graphs.
-func diffGraphs(before, after *core.BuildState) map[*core.BuildTarget]struct{} {
-	configChanged := !bytes.Equal(before.Hashes.Config, after.Hashes.Config)
-	log.Debugf("Has config changed between before and after build states: %v", configChanged)
-
+func diffGraphs(before *GraphSummary, after *core.BuildState) map[*core.BuildTarget]struct{} {
 	changed := map[*core.BuildTarget]struct{}{}
-	for _, afterTarget := range after.Graph.AllTargets() {
-		if beforeTarget := before.Graph.Target(afterTarget.Label); beforeTarget == nil || targetChanged(before, after, beforeTarget, afterTarget) || configChanged {
-			changed[afterTarget] = struct{}{}
+	// Shortcut everything if these aren't the same
+	if !bytes.Equal(before.configHash, after.Hashes.Config) {
+		log.Debug("Config has changed, emitting every target")
+		for t := range after.Graph.IterTargets() {
+			changed[t] = struct{}{}
+		}
+		return changed
+	}
+
+	targets := slices.Collect(after.Graph.IterTargets())
+	chunks := slices.Collect(slices.Chunk(targets, max(len(targets)/runtime.NumCPU(), 100)))
+	changes := make([][]*core.BuildTarget, len(chunks))
+	var wg sync.WaitGroup
+	for i, chunk := range chunks {
+		wg.Go(func() {
+			chunkChanges := make([]*core.BuildTarget, 0, len(chunk))
+			for _, t := range chunk {
+				if _, present := before.targets[targetHash(after, t)]; !present {
+					chunkChanges = append(chunkChanges, t)
+				}
+			}
+			changes[i] = chunkChanges
+		})
+	}
+	wg.Wait()
+	for _, chunk := range changes {
+		for _, t := range chunk {
+			changed[t] = struct{}{}
 		}
 	}
 	return changed
@@ -87,37 +112,52 @@ func changedTargets(state *core.BuildState, files []string, changed map[*core.Bu
 	return ls
 }
 
-// targetChanged returns true if the given two targets are not equivalent.
-func targetChanged(s1, s2 *core.BuildState, t1, t2 *core.BuildTarget) bool {
-	h1 := build.RuleHash(s1, t1, true, false)
-	h2 := build.RuleHash(s2, t2, true, false)
-	if !bytes.Equal(h1, h2) {
-		return true
+// targetHash returns a combined hash of the target which we use to check if they have changed.
+func targetHash(state *core.BuildState, target *core.BuildTarget) (ret [sha1.Size]byte) {
+	h := sha1.New()
+	// TODO(peterebden): We could slightly improve this by having an interface to pass our own hasher in instead of hashing this twice.
+	h.Write(build.RuleHash(state, target, true, false))
+	// The only thing we need to check source-wise is how tool paths resolve outside the repo.
+	// Changes to any tools within the repo are caught here anyway.
+	for _, tool := range target.AllTools() {
+		if _, ok := tool.Label(); !ok {
+			h.Write([]byte(tool.String()))
+		}
 	}
-	h1, err1 := sourceHash(s1, t1)
-	h2, err2 := sourceHash(s2, t2)
-	return !bytes.Equal(h1, h2) || err1 != nil || err2 != nil
+	h.Sum(ret[:0])
+	return ret
 }
 
-// sourceHash performs a partial source hash on a target to determine if it's changed.
-// This is a bit different to the one in the build package since we can't assume everything is
-// necessarily present (and for performance reasons don't want to hash *everything*).
-func sourceHash(state *core.BuildState, target *core.BuildTarget) ([]byte, error) {
-	var hash []byte
-	for _, tool := range target.AllTools() {
-		if _, ok := tool.Label(); ok {
-			continue // Skip in-repo tools, that will be handled via revdeps.
+// Summarise calculates a summary of a build state that is sufficient for change calculation, allowing the caller to free the 'before' state.
+func Summarise(state *core.BuildState) *GraphSummary {
+	targets := slices.Collect(state.Graph.IterTargets())
+	chunks := slices.Collect(slices.Chunk(targets, max(len(targets)/runtime.NumCPU(), 100)))
+	hashes := make([][][sha1.Size]byte, len(chunks))
+	var wg sync.WaitGroup
+	for i, chunk := range chunks {
+		wg.Go(func() {
+			chunkHashes := make([][sha1.Size]byte, len(chunk))
+			for j, t := range chunk {
+				chunkHashes[j] = targetHash(state, t)
+			}
+			hashes[i] = chunkHashes
+		})
+	}
+	wg.Wait()
+	m := map[[sha1.Size]byte]struct{}{}
+	for _, chunk := range hashes {
+		for _, h := range chunk {
+			m[h] = struct{}{}
 		}
-		// Tools outside the repo shouldn't change, so hashing the resolved tool path is enough.
-		hash = append(hash, toolPathHash(state, tool)...)
 	}
-	return hash, nil
+	return &GraphSummary{
+		configHash: state.Hashes.Config,
+		targets:    m,
+	}
 }
 
-func toolPathHash(state *core.BuildState, tool core.BuildInput) (hash []byte) {
-	h := sha1.New()
-	for _, path := range tool.LocalPaths(state.Graph) {
-		h.Write([]byte(path))
-	}
-	return h.Sum(nil)
+// A GraphSummary holds a summary of a build graph that is sufficient for change calculation
+type GraphSummary struct {
+	configHash []byte
+	targets    map[[sha1.Size]byte]struct{}
 }

src/query/changes_benchmark_test.go

@@ -0,0 +1,113 @@
+package query
+
+import (
+	"fmt"
+	"math/rand"
+	"testing"
+
+	"gopkg.in/op/go-logging.v1"
+
+	"github.com/thought-machine/please/src/core"
+)
+
+func BenchmarkDiffGraphs(b *testing.B) {
+	logging.SetLevel(logging.WARNING, "")
+	for _, numPackages := range []int{1000, 10000} {
+		before := createSyntheticGraph(numPackages)
+		summary := Summarise(before)
+		after := createSyntheticGraph(numPackages)
+		numTargets := len(after.Graph.AllTargets())
+
+		b.Run(fmt.Sprintf("Identical/%d", numTargets), func(b *testing.B) {
+			b.ReportAllocs()
+			for b.Loop() {
+				diffGraphs(summary, after)
+			}
+		})
+
+		// Modify roughly 1% of the targets in the 'after' graph.
+		changed := createSyntheticGraph(numPackages)
+		for i, t := range changed.Graph.AllTargets() {
+			if i%100 == 0 {
+				t.Command += " && echo changed"
+			}
+		}
+		b.Run(fmt.Sprintf("SomeChanged/%d", numTargets), func(b *testing.B) {
+			b.ReportAllocs()
+			for b.Loop() {
+				diffGraphs(summary, changed)
+			}
+		})
+	}
+}
+
+func BenchmarkSummarise(b *testing.B) {
+	logging.SetLevel(logging.WARNING, "")
+	for _, numPackages := range []int{1000, 10000, 100000} {
+		b.Run(fmt.Sprintf("Size/%d", numPackages), func(b *testing.B) {
+			graph := createSyntheticGraph(numPackages)
+			b.ReportAllocs()
+			b.ResetTimer()
+			for b.Loop() {
+				Summarise(graph)
+			}
+		})
+	}
+}
+
+// createSyntheticGraph creates a build state with a graph that looks vaguely like a real repo.
+// It is deterministic, so calling it twice with the same arguments produces equivalent graphs.
+// Each package contains a library (with a couple of hidden intermediate rules), a test, and a binary;
+// libraries depend on a handful of libraries in earlier packages.
+func createSyntheticGraph(numPackages int) *core.BuildState {
+	state := core.NewDefaultBuildState()
+	r := rand.New(rand.NewSource(42))
+	components := []string{
+		"src", "main", "cmd", "tools", "utils", "common", "query", "process", "update",
+		"run", "build", "assets", "frontend", "backend", "worker",
+	}
+	libs := make([]core.BuildLabel, 0, numPackages)
+	addTarget := func(pkg *core.Package, name string) *core.BuildTarget {
+		t := core.NewBuildTarget(core.BuildLabel{PackageName: pkg.Name, Name: name})
+		t.Command = fmt.Sprintf("$TOOLS_GO tool compile -o $OUT -p %s $SRCS", pkg.Name)
+		t.AddOutput(name + ".a")
+		t.AddLabel("go")
+		t.AddTool(core.SystemPathLabel{Name: "go", Path: state.Config.Path()})
+		t.Visibility = []core.BuildLabel{core.WholeGraph[0]}
+		state.Graph.AddTarget(t)
+		pkg.AddTarget(t)
+		return t
+	}
+	for i := 0; i < numPackages; i++ {
+		name := fmt.Sprintf("%s/%s/pkg%d", components[r.Intn(len(components))], components[r.Intn(len(components))], i)
+		pkg := core.NewPackage(name)
+		state.Graph.AddPackage(pkg)
+		base := components[r.Intn(len(components))]
+
+		srcs := addTarget(pkg, "_"+base+"#srcs")
+		for j := 0; j < 1+r.Intn(10); j++ {
+			srcs.AddSource(core.FileLabel{File: fmt.Sprintf("%s_%d.go", base, j), Package: name})
+		}
+		asm := addTarget(pkg, "_"+base+"#asm")
+		asm.AddDependency(srcs.Label)
+		lib := addTarget(pkg, base)
+		lib.AddSource(srcs.Label)
+		lib.AddDependency(srcs.Label)
+		lib.AddDependency(asm.Label)
+		for j := 0; j < len(libs) && j < 1+r.Intn(8); j++ {
+			lib.AddDependency(libs[r.Intn(len(libs))])
+		}
+		libs = append(libs, lib.Label)
+
+		test := addTarget(pkg, base+"_test")
+		test.AddSource(core.FileLabel{File: base + "_test.go", Package: name})
+		test.AddDependency(lib.Label)
+		test.Test = &core.TestFields{Command: "$TEST"}
+
+		bin := addTarget(pkg, base+"_bin")
+		bin.AddSource(core.FileLabel{File: "main.go", Package: name})
+		bin.AddDependency(lib.Label)
+		bin.IsBinary = true
+	}
+	return state
+}