From 524bd85dfd9c2c9fac263a2806e17cedb3cea85c Mon Sep 17 00:00:00 2001 From: Pierre Gimalac Date: Mon, 2 Mar 2026 01:34:33 +0100 Subject: 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 Reviewed-by: Sean Liao Reviewed-by: David Chase Auto-Submit: Sean Liao Reviewed-by: Michael Matloob TryBot-Bypass: Michael Matloob --- src/cmd/go/internal/modload/buildlist.go | 12 ++++- src/cmd/go/internal/modload/load.go | 12 ++++- .../script/work_sync_check_multiple_paths_once.txt | 56 ++++++++++++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 src/cmd/go/testdata/script/work_sync_check_multiple_paths_once.txt (limited to 'src') 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 -- cgit v1.3