cmd/go: respect -cpu=n on workers

[klauspost]

What version of Go are you using (go version)?

λ gotip version
go version devel go1.17-5542c10fbf Fri Jun 4 03:07:33 2021 +0000 windows/amd64

Does this issue reproduce with the latest release?

Tip branch build. Fuzzing not in latest release.

What operating system and processor architecture are you using (go env)?

go env Output

λ gotip env                                                                                                                                  
set GO111MODULE=                                                                                                                             
set GOARCH=amd64                                                                                                                             
set GOBIN=                                                                                                                                   
set GOCACHE=d:\temp\gocache                                                                                                                  
set GOENV=C:\Users\klaus\AppData\Roaming\go\env                                                                                              
set GOEXE=.exe                                                                                                                               
set GOFLAGS=                                                                                                                                 
set GOHOSTARCH=amd64                                                                                                                         
set GOHOSTOS=windows                                                                                                                         
set GOINSECURE=                                                                                                                              
set GOMODCACHE=e:\gopath\pkg\mod                                                                                                             
set GONOPROXY=                                                                                                                               
set GONOSUMDB=                                                                                                                               
set GOOS=windows                                                                                                                             
set GOPATH=e:\gopath                                                                                                                         
set GOPRIVATE=                                                                                                                               
set GOPROXY=https://goproxy.io                                                                                                               
set GOROOT=C:\Users\klaus\sdk\gotip                                                                                                          
set GOSUMDB=sum.golang.org                                                                                                                   
set GOTMPDIR=                                                                                                                                
set GOTOOLDIR=C:\Users\klaus\sdk\gotip\pkg\tool\windows_amd64                                                                                
set GOVCS=                                                                                                                                   
set GOVERSION=devel go1.17-5542c10fbf Fri Jun 4 03:07:33 2021 +0000                                                                          
set GCCGO=gccgo                                                                                                                              
set AR=ar                                                                                                                                    
set CC=gcc                                                                                                                                   
set CXX=g++                                                                                                                                  
set CGO_ENABLED=1                                                                                                                            
set GOMOD=NUL                                                                                                                                
set CGO_CFLAGS=-g -O2                                                                                                                        
set CGO_CPPFLAGS=                                                                                                                            
set CGO_CXXFLAGS=-g -O2                                                                                                                      
set CGO_FFLAGS=-g -O2                                                                                                                        
set CGO_LDFLAGS=-g -O2                                                                                                                       
set PKG_CONFIG=pkg-config                                                                                                                    
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=d:\temp\wintemp\go-build102784969=/tmp/go-build -gno-record-gcc-switches 

What did you do?

Run fuzz test which has a concurrent component, or pretty much any fuzz test.

with λ gotip test -fuzz=. -test.run=None -parallel=16 -cpu=2.

What did you expect to see?

I would expect each worker to have runtime.GOMAXPROCS(2) similar to how tests and benchmarks behave.

Arguments could be made for workers defaulting to runtime.GOMAXPROCS(1), but I would at least like the option to change it easily.

What did you see instead?

Each worker has default GOMAXPROCS, leading to significant CPU overprovisioning and workers fighting for CPU threads.

[katiehockman]

/cc @jayconrod

[rolandshoemaker]

Each worker is essentially single threaded, but I guess if the code being fuzzed is doing parallelized work it could lead to resource contention. Based on my experience I've only seen a drop in performance when I set -parallel > the number of available threads.

We could default each worker process to use runtime.NumCPU()/parallel CPUs. Allowing the user to set their own limit seems reasonable.

[klauspost]

We could default each worker process to use runtime.NumCPU()/parallel CPUs. Allowing the user to set their own limit seems reasonable.

@rolandshoemaker parallel makes no sense, that is mixing apples and oranges.

It currently uses the default, which I argue also doesn't make much sense, or at least should be controllable by the existing -cpu parameter on tests, so you can run fuzz tests with different GOMAXPROCS settings.

[rolandshoemaker]

@rolandshoemaker parallel makes no sense, that is mixing apples and oranges.

Why not? parallel controls the number of workers. If we want to prevent significant thread contention by default we can set each worker to use number of threads / number of workers threads. This should be user tunable, but we also need to set a reasonable default.

[klauspost]

@rolandshoemaker I misread your / as "or" (language meaning), not "divided by".

In my view -parallel controls the number of worker processes spawned, and has little to do with the concurrency I want to have for each.

While I understand your intent, we already have a parameter that controls concurrency, so when I use go test -cpu=4 I know my tests will be run with GOMAXPROCS(4). Your proposal gives GOMAXPROCS(1) with default settings, which is very reasonable, though TBH it seems a bit to magic for me.

[arp242]

A documentation update to the -cpu flag would improve things IMHO; the -cpu doesn't mention it, and while it's documented in -parallel it didn't occur to me to check there, and even when reading the docs for -cpu and -parallel it isn't super-clear how they interact when fuzzing. The only reason I figured this out is because I found this issue. And/or mentioning it in help fuzz probably would be a bad idea either.

I can see how it makes sense to use -parallel, but I agree with Klaus that this is counter-intuitive. I think a lot of people will run in to this. Better documentation should probably fix most of the confusion.

[katiehockman]

For now, we are going to disable -cpu while fuzzing (i.e. if -fuzz is set): it'll be a no-op. We will document that if folks want to adjust this, they can just set GOMAXPROCS in their environment. Maybe in future releases we can do more.

[gopherbot]

Change https://golang.org/cl/350154 mentions this issue: [dev.fuzz] testing, cmd/go: clarify documentation

[gopherbot]

Change https://golang.org/cl/351113 mentions this issue: testing, cmd/go: clarify documentation

[katiehockman]

Now that the documentation updates are in, I removed the release-blocker label and we can use this issue as discussion for future changes that may be made to support -cpu.