From: Bryan C. Mills Date: Thu, 16 Jun 2022 22:02:11 +0000 (-0400) Subject: cmd/go/internal/modindex: avoid walking modules when not needed X-Git-Tag: go1.19rc1~92 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=530511bacccdea0bb8a0fec644887c2613535c50;p=gostls13.git cmd/go/internal/modindex: avoid walking modules when not needed Due to a missed condition in CL 412394, we were walking all modules (instead of just the ones contained in GOROOT) at each invocation of a devel version of cmd/go. Moreover, while we were running cmd/go tests, we were re-walking GOROOT at each 'go' invocation in the test even though we expect GOROOT to be stable within a test run. This change always avoids walking non-GOROOT modules, and also adds a salt (configurable via GODEBUG) and uses it to avoid walking GOROOT modules when GOROOT is known to be stable (such as over the course of a 'cmd/go' test run). This should fix the cmd/go test timeouts that are currently occurring on the dragonfly-amd64 builder, such as this one: https://build.golang.org/log/21c01c3ae5490d387d84abeaf872b3a0a76ab8e5 Updates #53290. Change-Id: Ic807d215831a3cd21c63e0bccd3d7acd10d8f2b7 Reviewed-on: https://go-review.googlesource.com/c/go/+/412779 TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills Reviewed-by: Michael Matloob --- diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index b39a62f3e4..bbcecf2b2e 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -18,6 +18,7 @@ import ( "io" "io/fs" "log" + "math/rand" "os" "os/exec" "path/filepath" @@ -128,6 +129,19 @@ func TestMain(m *testing.M) { } os.Setenv("CMDGO_TEST_RUN_MAIN", "true") + if strings.HasPrefix(runtime.Version(), "devel ") && godebug.Get("goindexsalt") == "" { + // We're going to execute a lot of cmd/go tests, so set a consistent salt + // via GODEBUG so that the modindex package can avoid walking an entire + // GOROOT module whenever it tries to use an index for that module. + indexSalt := rand.Int63() + v := os.Getenv("GODEBUG") + if v == "" { + os.Setenv("GODEBUG", fmt.Sprintf("goindexsalt=%d", indexSalt)) + } else { + os.Setenv("GODEBUG", fmt.Sprintf("%s,goindexsalt=%d", v, indexSalt)) + } + } + // $GO_GCFLAGS a compiler debug flag known to cmd/dist, make.bash, etc. // It is not a standard go command flag; use os.Getenv, not cfg.Getenv. if os.Getenv("GO_GCFLAGS") != "" { diff --git a/src/cmd/go/internal/modindex/read.go b/src/cmd/go/internal/modindex/read.go index ffa091df41..e5761af679 100644 --- a/src/cmd/go/internal/modindex/read.go +++ b/src/cmd/go/internal/modindex/read.go @@ -12,11 +12,11 @@ import ( "go/build" "go/build/constraint" "go/token" + "internal/godebug" "internal/goroot" "internal/unsafeheader" "io/fs" "math" - "os" "path" "path/filepath" "runtime" @@ -39,15 +39,7 @@ import ( // It will be removed before the release. // TODO(matloob): Remove enabled once we have more confidence on the // module index. -var enabled = func() bool { - debug := strings.Split(os.Getenv("GODEBUG"), ",") - for _, f := range debug { - if f == "goindex=0" { - return false - } - } - return true -}() +var enabled bool = godebug.Get("goindex") != "0" // ModuleIndex represents and encoded module index file. It is used to // do the equivalent of build.Import of packages in the module and answer other @@ -61,26 +53,34 @@ type ModuleIndex struct { var fcache par.Cache +var salt = godebug.Get("goindexsalt") + func moduleHash(modroot string, ismodcache bool) (cache.ActionID, error) { // We expect modules stored within the module cache to be checksummed and - // immutable, and we expect released Go modules to change only infrequently - // (when the Go version changes). - if !ismodcache || !str.HasFilePathPrefix(modroot, cfg.GOROOT) { + // immutable, and we expect released modules within GOROOT to change only + // infrequently (when the Go version changes). + if !ismodcache && !str.HasFilePathPrefix(modroot, cfg.GOROOT) { + // The contents of this module may change over time. We don't want to pay + // the cost to detect changes and re-index whenever they occur, so just + // don't index it at all. return cache.ActionID{}, ErrNotIndexed } h := cache.NewHash("moduleIndex") - fmt.Fprintf(h, "module index %s %s %v\n", runtime.Version(), indexVersion, modroot) + fmt.Fprintf(h, "module index %s %s %s %v\n", runtime.Version(), salt, indexVersion, modroot) - if strings.HasPrefix(runtime.Version(), "devel ") { + if str.HasFilePathPrefix(modroot, cfg.GOROOT) && strings.HasPrefix(runtime.Version(), "devel ") && salt == "" { // This copy of the standard library is a development version, not a - // release. It could be based on a Git commit (like "devel go1.19-2a78e8afc0 - // Wed Jun 15 00:06:24 2022 +0000") with or without changes on top of that - // commit, or it could be completly artificial due to lacking a `git` binary - // (like "devel gomote.XXXXX", as synthesized by "gomote push" as of - // 2022-06-15). Compute an inexpensive hash of its files using mtimes so - // that during development we can continue to exercise the logic for cached - // GOROOT indexes. + // release. It could be based on a Git commit (like + // "devel go1.19-2a78e8afc0 Wed Jun 15 00:06:24 2022 +0000") with or + // without changes on top of that commit, or it could be completly + // artificial due to lacking a `git` binary (like "devel gomote.XXXXX", as + // synthesized by "gomote push" as of 2022-06-15). + // + // If the user provided a unique salt via GODEBUG, we can trust that it is + // unique and just go with it. Otherwise, we compute an inexpensive hash of + // its files using mtimes so that during development we can continue to + // exercise the logic for cached GOROOT indexes. // // mtimes may be granular, imprecise, and loosely updated (see // https://apenwarr.ca/log/20181113), but we don't expect Go contributors to