cmd/go: avoid redundant work in go work sync

First, updateWorkspaceRoots creates a new Requirements to update the
direct map, but always with an empty graph cache, discarding the cached
module graph. Since workspace roots do not change during package
loading, the graph is identical each time; propagate the cached graph to
the new Requirements to avoid rebuilding it from scratch on each call to
Graph.

Second, checkMultiplePaths iterates the full build list on every
loadFromRoots call. Its result depends only on the build list and
workspace replace directives, both fixed for the lifetime of the module
graph. Updated the code to only run once per graph.

Together these reduce go work sync time on a workspace with 184 modules
from ~35s to ~8.5s.

Fixes #77889

Change-Id: I7276aed3109424b3b74a4064b369ed7cbfca7735
Reviewed-on: https://go-review.googlesource.com/c/go/+/750500
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Bypass: Michael Matloob <matloob@golang.org>

3 files changed, 77 insertions, 3 deletions

diff --git a/src/cmd/go/internal/modload/buildlist.go b/src/cmd/go/internal/modload/buildlist.go
index a17626e3f5..73e881ef3c 100644
--- a/src/cmd/go/internal/modload/buildlist.go
+++ b/src/cmd/go/internal/modload/buildlist.go
@@ -295,6 +295,10 @@ type ModuleGraph struct {
 
 	buildListOnce sync.Once
 	buildList     []module.Version
+
+	// checkPathsOnce ensures checkMultiplePaths runs at most once per graph.
+	// Errors are checked and reported only on the first call.
+	checkPathsOnce sync.Once
 }
 
 var readModGraphDebugOnce sync.Once
@@ -797,7 +801,13 @@ func updateWorkspaceRoots(ld *Loader, ctx context.Context, direct map[string]boo
 		// return an error.
 		panic("add is not empty")
 	}
-	return newRequirements(ld, workspace, rs.rootModules, direct), nil
+	newRS := newRequirements(ld, workspace, rs.rootModules, direct)
+	// The root modules are unchanged (only the direct imports change),
+	// so the module graph can be reused to avoid rebuilding it from scratch.
+	if cached := rs.graph.Load(); cached != nil {
+		newRS.graphOnce.Do(func() { newRS.graph.Store(cached) })
+	}
+	return newRS, nil
 }
 
 // tidyPrunedRoots returns a minimal set of root requirements that maintains the
diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go
index 4efecdf8a1..4706d05e91 100644
--- a/src/cmd/go/internal/modload/load.go
+++ b/src/cmd/go/internal/modload/load.go
@@ -2020,13 +2020,21 @@ func (pld *packageLoader) computePatternAll() (all []string) {
 //
 // (See https://golang.org/issue/26607 and https://golang.org/issue/34650.)
 func (pld *packageLoader) checkMultiplePaths(ld *Loader) {
-	mods := pld.requirements.rootModules
 	if cached := pld.requirements.graph.Load(); cached != nil {
 		if mg := cached.mg; mg != nil {
-			mods = mg.BuildList()
+			// The check depends only on the build list and workspace replace
+			// directives, both fixed for the lifetime of mg, so skip it on
+			// subsequent calls sharing the same graph.
+			mg.checkPathsOnce.Do(func() {
+				checkMultiplePathsUncached(ld, pld, mg.BuildList())
+			})
+			return
 		}
 	}
+	checkMultiplePathsUncached(ld, pld, pld.requirements.rootModules)
+}
 
+func checkMultiplePathsUncached(ld *Loader, pld *packageLoader, mods []module.Version) {
 	firstPath := map[module.Version]string{}
 	for _, mod := range mods {
 		src := resolveReplacement(ld, mod)
diff --git a/src/cmd/go/testdata/script/work_sync_check_multiple_paths_once.txt b/src/cmd/go/testdata/script/work_sync_check_multiple_paths_once.txt
new file mode 100644
index 0000000000..bc2fd1f120
--- /dev/null
+++ b/src/cmd/go/testdata/script/work_sync_check_multiple_paths_once.txt
@@ -0,0 +1,56 @@
+# Test that checkMultiplePaths reports errors exactly once in a workspace with
+# multiple modules, even though it is called once per workspace module.
+#
+# example.com/a and example.com/b are both replaced by ../shared (which
+# canonicalizes to the same path in workspace mode), triggering a "used for
+# two different module paths" error. With three workspace modules the check
+# is called three times; the error should appear in stderr exactly once.
+
+go work sync
+stderr -count=1 'used for two different module paths'
+
+-- go.work --
+go 1.18
+
+use (
+	./m
+	./n
+	./o
+)
+
+-- m/go.mod --
+module example.com/m
+
+go 1.18
+
+require example.com/a v1.0.0
+
+replace example.com/a v1.0.0 => ../shared
+
+-- m/m.go --
+package m
+
+-- n/go.mod --
+module example.com/n
+
+go 1.18
+
+require example.com/b v1.0.0
+
+replace example.com/b v1.0.0 => ../shared
+
+-- n/n.go --
+package n
+
+-- o/go.mod --
+module example.com/o
+
+go 1.18
+
+-- o/o.go --
+package o
+
+-- shared/go.mod --
+module example.com/shared
+
+go 1.18