-
Notifications
You must be signed in to change notification settings - Fork 721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tools/ut: add a parallel parameter #8186
Changes from all commits
ad3ac3b
04241ca
f3d6c05
a0ced72
f42fc5e
3c5957c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,8 +95,7 @@ var ( | |
|
||
var ( | ||
// runtime | ||
p int | ||
buildParallel int | ||
parallel int | ||
workDir string | ||
coverFileTempDir string | ||
// arguments | ||
|
@@ -108,6 +107,7 @@ var ( | |
|
||
func main() { | ||
race = handleFlag("--race") | ||
parallelStr := stripFlag("--parallel") | ||
junitFile = stripFlag("--junitfile") | ||
coverProfile = stripFlag("--coverprofile") | ||
ignoreDir = stripFlag("--ignore") | ||
|
@@ -122,11 +122,21 @@ func main() { | |
defer os.RemoveAll(coverFileTempDir) | ||
} | ||
|
||
// Get the correct count of CPU if it's in docker. | ||
p = runtime.GOMAXPROCS(0) | ||
// We use 2 * p for `go build` to make it faster. | ||
buildParallel = p * 2 | ||
var err error | ||
procs := runtime.GOMAXPROCS(0) | ||
if parallelStr == "" { | ||
// Get the correct count of CPU if it's in docker. | ||
parallel = procs | ||
} else { | ||
parallel, err = strconv.Atoi(parallelStr) | ||
if err != nil { | ||
fmt.Println("parse parallel error", err) | ||
return | ||
} | ||
if parallel > procs { | ||
fmt.Printf("Recommend to set parallel be same as the GOMAXPROCS=%d\n", procs) | ||
} | ||
} | ||
workDir, err = os.Getwd() | ||
if err != nil { | ||
fmt.Println("os.Getwd() error", err) | ||
|
@@ -353,12 +363,12 @@ func cmdRun(args ...string) bool { | |
} | ||
} | ||
|
||
fmt.Printf("building task finish, parallelism=%d, count=%d, takes=%v\n", buildParallel, len(tasks), time.Since(start)) | ||
fmt.Printf("building task finish, parallelism=%d, count=%d, takes=%v\n", parallel*2, len(tasks), time.Since(start)) | ||
|
||
taskCh := make(chan task, 100) | ||
works := make([]numa, p) | ||
works := make([]numa, parallel) | ||
var wg sync.WaitGroup | ||
for i := 0; i < p; i++ { | ||
for i := 0; i < parallel; i++ { | ||
wg.Add(1) | ||
go works[i].worker(&wg, taskCh) | ||
} | ||
|
@@ -400,7 +410,7 @@ func cmdRun(args ...string) bool { | |
|
||
// stripFlag strip the '--flag xxx' from the command line os.Args | ||
// Example of the os.Args changes | ||
// Before: ut run pkg TestXXX --coverprofile xxx --junitfile yyy | ||
// Before: ut run pkg TestXXX --coverprofile xxx --junitfile yyy --parallel 16 | ||
// After: ut run pkg TestXXX | ||
// The value of the flag is returned. | ||
func stripFlag(flag string) string { | ||
|
@@ -636,7 +646,7 @@ func (*numa) testCommand(pkg string, fn string) *exec.Cmd { | |
args = append(args, "-test.coverprofile", tmpFile) | ||
} | ||
if strings.Contains(fn, "Suite") { | ||
args = append(args, "-test.cpu", fmt.Sprint(p/2)) | ||
args = append(args, "-test.cpu", fmt.Sprint(parallel/2)) | ||
} else { | ||
args = append(args, "-test.cpu", "1") | ||
} | ||
|
@@ -705,7 +715,8 @@ func buildTestBinaryMulti(pkgs []string) error { | |
packages = append(packages, path.Join(modulePath, pkg)) | ||
} | ||
|
||
p := strconv.Itoa(buildParallel) | ||
// We use 2 * parallel for `go build` to make it faster. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One question, why do we choose There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. background: When building the test binaries, by default it is the number of cores(pkg.go.dev/cmd/go#hdr-Compile_packages_and_dependencies):
When running the 'building binaries stage' of make ut, the CPU usage is not high with the default value. So I think it's a better idea to increase the parallelism.
Choice 2 wasn't much thought or testing, and I think 2 is fine for the time being :) Or do you have any ideas? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I have no idea about that. I thought 2 had any special meaning before |
||
p := strconv.Itoa(parallel * 2) | ||
cmd := exec.Command("go", "test", "-p", p, "--exec", xprogPath, "-vet", "off", "--tags=tso_function_test,deadlock") | ||
if coverProfile != "" { | ||
coverpkg := "./..." | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need check if the
parallel<=runtime.GOMAXPROCS(0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need, we can support parallel greater than procs. It depends on the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to set a maximum value. For example, 1000. I just said it casually
what do you think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense, and add a hint when parallel was set greater than procs :)
f42fc5e#diff-d1de7e6511458d33a769c0c501ec0ea45494dd9d2e830dc315e8fe623eb34b76R132