]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/go: make 'mod verify' use multiple CPUs
authorDaniel Martí <mvdan@mvdan.cc>
Fri, 24 Apr 2020 09:53:17 +0000 (10:53 +0100)
committerDaniel Martí <mvdan@mvdan.cc>
Thu, 30 Apr 2020 07:05:55 +0000 (07:05 +0000)
'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í <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
src/cmd/go/internal/modcmd/verify.go

index ac3f1351c879eb0cc4a8eda938ce91d5b78ac8a6..b7fd7fa8e0194907160daa37fca0778797421473 100644 (file)
@@ -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
 }