Skip to content

Commit 38431f1

Browse files
committed
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 <rsc@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
1 parent ba23fa4 commit 38431f1

File tree

3 files changed

+53
-29
lines changed

3 files changed

+53
-29
lines changed

src/cmd/go/go_test.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -5263,8 +5263,14 @@ func TestCacheVet(t *testing.T) {
52635263
if strings.Contains(os.Getenv("GODEBUG"), "gocacheverify") {
52645264
t.Skip("GODEBUG gocacheverify")
52655265
}
5266-
if cfg.Getenv("GOCACHE") == "off" {
5267-
tooSlow(t)
5266+
if testing.Short() {
5267+
// In short mode, reuse cache.
5268+
// Test failures may be masked if the cache has just the right entries already
5269+
// (not a concern during all.bash, which runs in a clean cache).
5270+
if cfg.Getenv("GOCACHE") == "off" {
5271+
tooSlow(t)
5272+
}
5273+
} else {
52685274
tg.makeTempdir()
52695275
tg.setenv("GOCACHE", tg.path("cache"))
52705276
}

src/cmd/go/internal/work/action.go

+24-5
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,11 @@ type Action struct {
8484
actionID cache.ActionID // cache ID of action input
8585
buildID string // build ID of action output
8686

87-
VetxOnly bool // Mode=="vet": only being called to supply info about dependencies
88-
needVet bool // Mode=="build": need to fill in vet config
89-
vetCfg *vetConfig // vet config
90-
output []byte // output redirect buffer (nil means use b.Print)
87+
VetxOnly bool // Mode=="vet": only being called to supply info about dependencies
88+
needVet bool // Mode=="build": need to fill in vet config
89+
needBuild bool // Mode=="build": need to do actual build (can be false if needVet is true)
90+
vetCfg *vetConfig // vet config
91+
output []byte // output redirect buffer (nil means use b.Print)
9192

9293
// Execution state.
9394
pending int // number of deps yet to complete
@@ -212,6 +213,8 @@ const (
212213
ModeBuild BuildMode = iota
213214
ModeInstall
214215
ModeBuggyInstall
216+
217+
ModeVetOnly = 1 << 8
215218
)
216219

217220
func (b *Builder) Init() {
@@ -354,6 +357,9 @@ func (b *Builder) AutoAction(mode, depMode BuildMode, p *load.Package) *Action {
354357
// depMode is the action (build or install) to use when building dependencies.
355358
// To turn package main into an executable, call b.Link instead.
356359
func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Action {
360+
vetOnly := mode&ModeVetOnly != 0
361+
mode &^= ModeVetOnly
362+
357363
if mode != ModeBuild && (p.Internal.Local || p.Module != nil) && p.Target == "" {
358364
// Imported via local path or using modules. No permanent target.
359365
mode = ModeBuild
@@ -400,6 +406,19 @@ func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Actio
400406
return a
401407
})
402408

409+
// Find the build action; the cache entry may have been replaced
410+
// by the install action during (*Builder).installAction.
411+
buildAction := a
412+
switch buildAction.Mode {
413+
case "build", "built-in package":
414+
// ok
415+
case "build-install":
416+
buildAction = a.Deps[0]
417+
default:
418+
panic("lost build action: " + buildAction.Mode)
419+
}
420+
buildAction.needBuild = buildAction.needBuild || !vetOnly
421+
403422
// Construct install action.
404423
if mode == ModeInstall || mode == ModeBuggyInstall {
405424
a = b.installAction(a, mode)
@@ -421,7 +440,7 @@ func (b *Builder) VetAction(mode, depMode BuildMode, p *load.Package) *Action {
421440
func (b *Builder) vetAction(mode, depMode BuildMode, p *load.Package) *Action {
422441
// Construct vet action.
423442
a := b.cacheAction("vet", p, func() *Action {
424-
a1 := b.CompileAction(mode, depMode, p)
443+
a1 := b.CompileAction(mode|ModeVetOnly, depMode, p)
425444

426445
// vet expects to be able to import "fmt".
427446
var stk load.ImportStack

src/cmd/go/internal/work/exec.go

+21-22
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,8 @@ func (b *Builder) build(a *Action) (err error) {
367367
return 0
368368
}
369369

370-
cached := false
371-
need := bit(needBuild, !b.IsCmdList || b.NeedExport) |
370+
cachedBuild := false
371+
need := bit(needBuild, !b.IsCmdList && a.needBuild || b.NeedExport) |
372372
bit(needCgoHdr, b.needCgoHdr(a)) |
373373
bit(needVet, a.needVet) |
374374
bit(needCompiledGoFiles, b.NeedCompiledGoFiles)
@@ -377,23 +377,23 @@ func (b *Builder) build(a *Action) (err error) {
377377
if b.useCache(a, p, b.buildActionID(a), p.Target) {
378378
// We found the main output in the cache.
379379
// If we don't need any other outputs, we can stop.
380+
// Otherwise, we need to write files to a.Objdir (needVet, needCgoHdr).
381+
// Remember that we might have them in cache
382+
// and check again after we create a.Objdir.
383+
cachedBuild = true
384+
a.output = []byte{} // start saving output in case we miss any cache results
380385
need &^= needBuild
381386
if b.NeedExport {
382387
p.Export = a.built
383388
}
384389
if need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) {
385390
need &^= needCompiledGoFiles
386391
}
387-
// Otherwise, we need to write files to a.Objdir (needVet, needCgoHdr).
388-
// Remember that we might have them in cache
389-
// and check again after we create a.Objdir.
390-
cached = true
391-
a.output = []byte{} // start saving output in case we miss any cache results
392392
}
393393

394394
// Source files might be cached, even if the full action is not
395395
// (e.g., go list -compiled -find).
396-
if !cached && need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) {
396+
if !cachedBuild && need&needCompiledGoFiles != 0 && b.loadCachedSrcFiles(a) {
397397
need &^= needCompiledGoFiles
398398
}
399399

@@ -438,21 +438,20 @@ func (b *Builder) build(a *Action) (err error) {
438438
}
439439
objdir := a.Objdir
440440

441-
if cached {
442-
if need&needCgoHdr != 0 && b.loadCachedCgoHdr(a) {
443-
need &^= needCgoHdr
444-
}
441+
// Load cached cgo header, but only if we're skipping the main build (cachedBuild==true).
442+
if cachedBuild && need&needCgoHdr != 0 && b.loadCachedCgoHdr(a) {
443+
need &^= needCgoHdr
444+
}
445445

446-
// Load cached vet config, but only if that's all we have left
447-
// (need == needVet, not testing just the one bit).
448-
// If we are going to do a full build anyway,
449-
// we're going to regenerate the files below anyway.
450-
if need == needVet && b.loadCachedVet(a) {
451-
need &^= needVet
452-
}
453-
if need == 0 {
454-
return nil
455-
}
446+
// Load cached vet config, but only if that's all we have left
447+
// (need == needVet, not testing just the one bit).
448+
// If we are going to do a full build anyway,
449+
// we're going to regenerate the files below anyway.
450+
if need == needVet && b.loadCachedVet(a) {
451+
need &^= needVet
452+
}
453+
if need == 0 {
454+
return nil
456455
}
457456

458457
// make target directory

0 commit comments

Comments
 (0)