From b7e54d8d0784ceb21f7d4d406cf42c86a7fdb0f8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Daniel=20Mart=C3=AD?= Date: Fri, 24 Apr 2020 10:53:17 +0100 Subject: [PATCH] cmd/go: make 'mod verify' use multiple CPUs MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 'go mod verify' checksums one module zip at a time, which is CPU-intensive on most modern machines with fast disks. As a result, one can see a CPU bottleneck when running the command on, for example, a module where 'go list -m all' lists ~440 modules: $ /usr/bin/time go mod verify all modules verified 11.47user 0.77system 0:09.41elapsed 130%CPU (0avgtext+0avgdata 24284maxresident)k 0inputs+0outputs (0major+4156minor)pagefaults 0swaps Instead, verify up to GOMAXPROCS zips at once, which should line up pretty well with the amount of processors we can use on a machine. The results below are obtained via 'benchcmd -n 5 GoModVerify go mod verify' on the same large module. name old time/op new time/op delta GoModVerify 9.35s ± 1% 3.03s ± 2% -67.60% (p=0.008 n=5+5) name old user-time/op new user-time/op delta GoModVerify 11.2s ± 1% 16.3s ± 3% +45.38% (p=0.008 n=5+5) name old sys-time/op new sys-time/op delta GoModVerify 841ms ± 9% 865ms ± 8% ~ (p=0.548 n=5+5) name old peak-RSS-bytes new peak-RSS-bytes delta GoModVerify 27.8MB ±13% 50.7MB ±27% +82.01% (p=0.008 n=5+5) The peak memory usage nearly doubles, and there is some extra overhead, but it seems clearly worth the tradeoff given that we see a ~3x speedup on my laptop with 4 physical cores. The vast majority of developer machines nowadays should have 2-4 cores at least. No test or benchmark is included; one can benchmark 'go mod verify' directly, as I did above. The existing tests also cover correctness, including any data races via -race. Fixes #38623. Change-Id: I45d8154687a6f3a6a9fb0e2b13da4190f321246c Reviewed-on: https://go-review.googlesource.com/c/go/+/229817 Run-TryBot: Daniel Martí TryBot-Result: Gobot Gobot Reviewed-by: Bryan C. Mills --- src/cmd/go/internal/modcmd/verify.go | 55 ++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/src/cmd/go/internal/modcmd/verify.go b/src/cmd/go/internal/modcmd/verify.go index ac3f1351c8..b7fd7fa8e0 100644 --- a/src/cmd/go/internal/modcmd/verify.go +++ b/src/cmd/go/internal/modcmd/verify.go @@ -10,6 +10,7 @@ import ( "fmt" "io/ioutil" "os" + "runtime" "cmd/go/internal/base" "cmd/go/internal/cfg" @@ -52,17 +53,41 @@ func runVerify(cmd *base.Command, args []string) { base.Fatalf("go: cannot find main module; see 'go help modules'") } } + + // Only verify up to GOMAXPROCS zips at once. + type token struct{} + sem := make(chan token, runtime.GOMAXPROCS(0)) + + // Use a slice of result channels, so that the output is deterministic. + mods := modload.LoadBuildList()[1:] + errsChans := make([]<-chan []error, len(mods)) + + for i, mod := range mods { + sem <- token{} + errsc := make(chan []error, 1) + errsChans[i] = errsc + mod := mod // use a copy to avoid data races + go func() { + errsc <- verifyMod(mod) + <-sem + }() + } + ok := true - for _, mod := range modload.LoadBuildList()[1:] { - ok = verifyMod(mod) && ok + for _, errsc := range errsChans { + errs := <-errsc + for _, err := range errs { + base.Errorf("%s", err) + ok = false + } } if ok { fmt.Printf("all modules verified\n") } } -func verifyMod(mod module.Version) bool { - ok := true +func verifyMod(mod module.Version) []error { + var errs []error zip, zipErr := modfetch.CachePath(mod, "zip") if zipErr == nil { _, zipErr = os.Stat(zip) @@ -73,10 +98,10 @@ func verifyMod(mod module.Version) bool { if zipErr != nil && errors.Is(zipErr, os.ErrNotExist) && dirErr != nil && errors.Is(dirErr, os.ErrNotExist) { // Nothing downloaded yet. Nothing to verify. - return true + return nil } - base.Errorf("%s %s: missing ziphash: %v", mod.Path, mod.Version, err) - return false + errs = append(errs, fmt.Errorf("%s %s: missing ziphash: %v", mod.Path, mod.Version, err)) + return errs } h := string(bytes.TrimSpace(data)) @@ -85,11 +110,10 @@ func verifyMod(mod module.Version) bool { } else { hZ, err := dirhash.HashZip(zip, dirhash.DefaultHash) if err != nil { - base.Errorf("%s %s: %v", mod.Path, mod.Version, err) - return false + errs = append(errs, fmt.Errorf("%s %s: %v", mod.Path, mod.Version, err)) + return errs } else if hZ != h { - base.Errorf("%s %s: zip has been modified (%v)", mod.Path, mod.Version, zip) - ok = false + errs = append(errs, fmt.Errorf("%s %s: zip has been modified (%v)", mod.Path, mod.Version, zip)) } } if dirErr != nil && errors.Is(dirErr, os.ErrNotExist) { @@ -98,13 +122,12 @@ func verifyMod(mod module.Version) bool { hD, err := dirhash.HashDir(dir, mod.Path+"@"+mod.Version, dirhash.DefaultHash) if err != nil { - base.Errorf("%s %s: %v", mod.Path, mod.Version, err) - return false + errs = append(errs, fmt.Errorf("%s %s: %v", mod.Path, mod.Version, err)) + return errs } if hD != h { - base.Errorf("%s %s: dir has been modified (%v)", mod.Path, mod.Version, dir) - ok = false + errs = append(errs, fmt.Errorf("%s %s: dir has been modified (%v)", mod.Path, mod.Version, dir)) } } - return ok + return errs } -- 2.50.0