Skip to content

Commit 83a6c13

Browse files
qiulaidongfenggopherbot
authored andcommitted
cmd/dist: avoid CPU underutilization starting from GOMAXPROCS=2 runtime
This CL is doing now is: change maxbg to increase test parallelism. adjust test sequence. This CL speeds up the go tool dist test, most of the speed up is due to the fact that the three time-consuming tests cmd/internal/testdir and API check and runtime/race can be done in parallel with the GOMAXPROCS=2 runtime on a machine with enough CPU cores. In windows with an 8-core 16-thread CPU, this CL can complete all other tests before GOMAXPROCS=2 runtime -cpu=1,2,4 -quick completes. Fixes #65164 Change-Id: I56ed7031d58be3bece9f975bfc73e5c834d0a4fa GitHub-Last-Rev: 18cffb7 GitHub-Pull-Request: #65703 Reviewed-on: https://go-review.googlesource.com/c/go/+/563916 Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]> Commit-Queue: Ian Lance Taylor <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]> Reviewed-by: David Chase <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 8aeec7c commit 83a6c13

File tree

1 file changed

+51
-33
lines changed

1 file changed

+51
-33
lines changed

src/cmd/dist/test.go

+51-33
Original file line numberDiff line numberDiff line change
@@ -711,24 +711,6 @@ func (t *tester) registerTests() {
711711
})
712712
}
713713

714-
// Runtime CPU tests.
715-
if !t.compileOnly && t.hasParallelism() {
716-
for i := 1; i <= 4; i *= 2 {
717-
t.registerTest(fmt.Sprintf("GOMAXPROCS=2 runtime -cpu=%d -quick", i),
718-
&goTest{
719-
variant: "cpu" + strconv.Itoa(i),
720-
timeout: 300 * time.Second,
721-
cpu: strconv.Itoa(i),
722-
short: true,
723-
testFlags: []string{"-quick"},
724-
// We set GOMAXPROCS=2 in addition to -cpu=1,2,4 in order to test runtime bootstrap code,
725-
// creation of first goroutines and first garbage collections in the parallel setting.
726-
env: []string{"GOMAXPROCS=2"},
727-
pkg: "runtime",
728-
})
729-
}
730-
}
731-
732714
// GOEXPERIMENT=rangefunc tests
733715
if !t.compileOnly {
734716
t.registerTest("GOEXPERIMENT=rangefunc go test iter",
@@ -864,10 +846,6 @@ func (t *tester) registerTests() {
864846
})
865847
}
866848

867-
if t.raceDetectorSupported() {
868-
t.registerRaceTests()
869-
}
870-
871849
const cgoHeading = "Testing cgo"
872850
if t.cgoEnabled {
873851
t.registerCgoTests(cgoHeading)
@@ -883,6 +861,40 @@ func (t *tester) registerTests() {
883861
})
884862
}
885863

864+
// Only run the API check on fast development platforms.
865+
// Every platform checks the API on every GOOS/GOARCH/CGO_ENABLED combination anyway,
866+
// so we really only need to run this check once anywhere to get adequate coverage.
867+
// To help developers avoid trybot-only failures, we try to run on typical developer machines
868+
// which is darwin,linux,windows/amd64 and darwin/arm64.
869+
//
870+
// The same logic applies to the release notes that correspond to each api/next file.
871+
if goos == "darwin" || ((goos == "linux" || goos == "windows") && goarch == "amd64") {
872+
t.registerTest("API release note check", &goTest{variant: "check", pkg: "cmd/relnote", testFlags: []string{"-check"}})
873+
t.registerTest("API check", &goTest{variant: "check", pkg: "cmd/api", timeout: 5 * time.Minute, testFlags: []string{"-check"}})
874+
}
875+
876+
// Runtime CPU tests.
877+
if !t.compileOnly && t.hasParallelism() {
878+
for i := 1; i <= 4; i *= 2 {
879+
t.registerTest(fmt.Sprintf("GOMAXPROCS=2 runtime -cpu=%d -quick", i),
880+
&goTest{
881+
variant: "cpu" + strconv.Itoa(i),
882+
timeout: 300 * time.Second,
883+
cpu: strconv.Itoa(i),
884+
short: true,
885+
testFlags: []string{"-quick"},
886+
// We set GOMAXPROCS=2 in addition to -cpu=1,2,4 in order to test runtime bootstrap code,
887+
// creation of first goroutines and first garbage collections in the parallel setting.
888+
env: []string{"GOMAXPROCS=2"},
889+
pkg: "runtime",
890+
})
891+
}
892+
}
893+
894+
if t.raceDetectorSupported() {
895+
t.registerRaceTests()
896+
}
897+
886898
if goos != "android" && !t.iOS() {
887899
// Only start multiple test dir shards on builders,
888900
// where they get distributed to multiple machines.
@@ -907,17 +919,6 @@ func (t *tester) registerTests() {
907919
)
908920
}
909921
}
910-
// Only run the API check on fast development platforms.
911-
// Every platform checks the API on every GOOS/GOARCH/CGO_ENABLED combination anyway,
912-
// so we really only need to run this check once anywhere to get adequate coverage.
913-
// To help developers avoid trybot-only failures, we try to run on typical developer machines
914-
// which is darwin,linux,windows/amd64 and darwin/arm64.
915-
//
916-
// The same logic applies to the release notes that correspond to each api/next file.
917-
if goos == "darwin" || ((goos == "linux" || goos == "windows") && goarch == "amd64") {
918-
t.registerTest("API check", &goTest{variant: "check", pkg: "cmd/api", timeout: 5 * time.Minute, testFlags: []string{"-check"}})
919-
t.registerTest("API release note check", &goTest{variant: "check", pkg: "cmd/relnote", testFlags: []string{"-check"}})
920-
}
921922
}
922923

923924
// addTest adds an arbitrary test callback to the test list.
@@ -1313,6 +1314,23 @@ func (t *tester) runPending(nextTest *distTest) {
13131314
}(w)
13141315
}
13151316

1317+
maxbg := maxbg
1318+
// for runtime.NumCPU() < 4 || runtime.GOMAXPROCS(0) == 1, do not change maxbg.
1319+
// Because there is not enough CPU to parallel the testing of multiple packages.
1320+
if runtime.NumCPU() > 4 && runtime.GOMAXPROCS(0) != 1 {
1321+
for _, w := range worklist {
1322+
// See go.dev/issue/65164
1323+
// because GOMAXPROCS=2 runtime CPU usage is low,
1324+
// so increase maxbg to avoid slowing down execution with low CPU usage.
1325+
// This makes testing a single package slower,
1326+
// but testing multiple packages together faster.
1327+
if strings.Contains(w.dt.heading, "GOMAXPROCS=2 runtime") {
1328+
maxbg = runtime.NumCPU()
1329+
break
1330+
}
1331+
}
1332+
}
1333+
13161334
started := 0
13171335
ended := 0
13181336
var last *distTest

0 commit comments

Comments
 (0)