From 38431f1044880b936e35034ded19a6a8bc9faa21 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 9 May 2019 21:56:53 -0400 Subject: [PATCH] cmd/go: do not build test packages unnecessarily during go vet Vet needs export data for the imports of the package it is analyzing. Vet does not need export data for the package itself, since vet will do its own type checking. Assuming that vet is just as good as the compiler at detecting invalid programs, don't run the compiler unnecessarily. This especially matters for tests without external test files or for which the external test files do not import the test-augmented original package. In that case, the test-augmented original package need not be compiled at all. Cuts time for 'go clean -cache && go vet -x cmd/compile/internal/ssa' from 7.6r 24.3u 2.8s to 3.5r 8.5u 1.9s, by not running the compiler on the augmented test package. There is still more to be done here - if we do need to build a test-augmented package, we rerun cgo unnecessarily. But this is a big help. Cuts time for 'go vet std cmd' by about 30%. For #31916. Change-Id: If6136b4d384f1da77aed90b43f1a6b95f09b5d86 Reviewed-on: https://go-review.googlesource.com/c/go/+/176438 Run-TryBot: Russ Cox Reviewed-by: Brad Fitzpatrick --- src/cmd/go/go_test.go | 10 +++++-- src/cmd/go/internal/work/action.go | 29 ++++++++++++++++---- src/cmd/go/internal/work/exec.go | 43 +++++++++++++++--------------- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 1ec82ad532..49b0e6d255 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -5263,8 +5263,14 @@ func TestCacheVet(t *testing.T) { if strings.Contains(os.Getenv("GODEBUG"), "gocacheverify") { t.Skip("GODEBUG gocacheverify") } - if cfg.Getenv("GOCACHE") == "off" { - tooSlow(t) + if testing.Short() { + // In short mode, reuse cache. + // Test failures may be masked if the cache has just the right entries already + // (not a concern during all.bash, which runs in a clean cache). + if cfg.Getenv("GOCACHE") == "off" { + tooSlow(t) + } + } else { tg.makeTempdir() tg.setenv("GOCACHE", tg.path("cache")) } diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go index 7a74b1bb0d..f17137e666 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -84,10 +84,11 @@ type Action struct { actionID cache.ActionID // cache ID of action input buildID string // build ID of action output - VetxOnly bool // Mode=="vet": only being called to supply info about dependencies - needVet bool // Mode=="build": need to fill in vet config - vetCfg *vetConfig // vet config - output []byte // output redirect buffer (nil means use b.Print) + VetxOnly bool // Mode=="vet": only being called to supply info about dependencies + needVet bool // Mode=="build": need to fill in vet config + needBuild bool // Mode=="build": need to do actual build (can be false if needVet is true) + vetCfg *vetConfig // vet config + output []byte // output redirect buffer (nil means use b.Print) // Execution state. pending int // number of deps yet to complete @@ -212,6 +213,8 @@ const ( ModeBuild BuildMode = iota ModeInstall ModeBuggyInstall + + ModeVetOnly = 1 << 8 ) func (b *Builder) Init() { @@ -354,6 +357,9 @@ func (b *Builder) AutoAction(mode, depMode BuildMode, p *load.Package) *Action { // depMode is the action (build or install) to use when building dependencies. // To turn package main into an executable, call b.Link instead. func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Action { + vetOnly := mode&ModeVetOnly != 0 + mode &^= ModeVetOnly + if mode != ModeBuild && (p.Internal.Local || p.Module != nil) && p.Target == "" { // Imported via local path or using modules. No permanent target. mode = ModeBuild @@ -400,6 +406,19 @@ func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Actio return a }) + // Find the build action; the cache entry may have been replaced + // by the install action during (*Builder).installAction. + buildAction := a + switch buildAction.Mode { + case "build", "built-in package": + // ok + case "build-install": + buildAction = a.Deps[0] + default: + panic("lost build action: " + buildAction.Mode) + } + buildAction.needBuild = buildAction.needBuild || !vetOnly + // Construct install action. if mode == ModeInstall || mode == ModeBuggyInstall { a = b.installAction(a, mode) @@ -421,7 +440,7 @@ func (b *Builder) VetAction(mode, depMode BuildMode, p *load.Package) *Action { func (b *Builder) vetAction(mode, depMode BuildMode, p *load.Package) *Action { // Construct vet action. a := b.cacheAction("vet", p, func() *Action { - a1 := b.CompileAction(mode, depMode, p) + a1 := b.CompileAction(mode|ModeVetOnly, depMode, p) // vet expects to be able to import "fmt". var stk load.ImportStack diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 6f8dca9b89..9381bd6f1e 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -367,8 +367,8 @@ func (b *Builder) build(a *Action) (err error) { return 0 } - cached := false - need := bit(needBuild, !b.IsCmdList || b.NeedExport) | + cachedBuild := false + need := bit(needBuild, !b.IsCmdList && a.needBuild || b.NeedExport) | bit(needCgoHdr, b.needCgoHdr(a)) | bit(needVet, a.needVet) | bit(needCompiledGoFiles, b.NeedCompiledGoFiles) @@ -377,6 +377,11 @@ func (b *Builder) build(a *Action) (err error) { if b.useCache(a, p, b.buildActionID(a), p.Target) { // We found the main output in the cache. // If we don't need any other outputs, we can stop. + // Otherwise, we need to write files to a.Objdir (needVet, needCgoHdr). + // Remember that we might have them in cache + // and check again after we create a.Objdir. + cachedBuild = true + a.output = []byte{} // start saving output in case we miss any cache results need &^= needBuild if b.NeedExport { p.Export = a.built @@ -384,16 +389,11 @@ func (b *Builder) build(a *Action) (err error) { if need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) { need &^= needCompiledGoFiles } - // Otherwise, we need to write files to a.Objdir (needVet, needCgoHdr). - // Remember that we might have them in cache - // and check again after we create a.Objdir. - cached = true - a.output = []byte{} // start saving output in case we miss any cache results } // Source files might be cached, even if the full action is not // (e.g., go list -compiled -find). - if !cached && need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) { + if !cachedBuild && need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) { need &^= needCompiledGoFiles } @@ -438,21 +438,20 @@ func (b *Builder) build(a *Action) (err error) { } objdir := a.Objdir - if cached { - if need&needCgoHdr != 0 && b.loadCachedCgoHdr(a) { - need &^= needCgoHdr - } + // Load cached cgo header, but only if we're skipping the main build (cachedBuild==true). + if cachedBuild && need&needCgoHdr != 0 && b.loadCachedCgoHdr(a) { + need &^= needCgoHdr + } - // Load cached vet config, but only if that's all we have left - // (need == needVet, not testing just the one bit). - // If we are going to do a full build anyway, - // we're going to regenerate the files below anyway. - if need == needVet && b.loadCachedVet(a) { - need &^= needVet - } - if need == 0 { - return nil - } + // Load cached vet config, but only if that's all we have left + // (need == needVet, not testing just the one bit). + // If we are going to do a full build anyway, + // we're going to regenerate the files below anyway. + if need == needVet && b.loadCachedVet(a) { + need &^= needVet + } + if need == 0 { + return nil } // make target directory -- 2.48.1