code review 5732057: runtime: wait for main goroutine before setting GOMAXPROCS.

runtime: wait for main goroutine before setting GOMAXPROCS.

Fixes issue 3182.

[remyoudompheng, 2012-03-04 10:29:49]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
remy@archlinux.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[dvyukov, 2012-03-05 07:12:15]

On 2012/03/04 10:29:49, remyoudompheng wrote:
> Hello mailto:golang-dev@googlegroups.com (cc:
mailto:golang-dev@googlegroups.com,
> mailto:remy@archlinux.org),
> 
> I'd like you to review this change to
> https://go.googlecode.com/hg/

I do not look into this in detail, but take into account that CPUs are not
threads. A dozen of worker threads can be multiplexed onto single CPU, so
setting maxcpu to 1 does not limit number of worker threads - main thread can
block in a syscall and then another thread will be created.

[remyoudompheng, 2012-03-05 07:20:47]

Yes, I don't really feel comfortable about this, so I hope someone can have a
look. I think it may still work, because between these two lines there is very
little code that runs (only newproc1, if I read correctly)...

[dvyukov, 2012-03-05 11:22:04]

On 2012/03/05 07:20:47, remyoudompheng wrote:
> Yes, I don't really feel comfortable about this, so I hope someone can have a
> look. I think it may still work, because between these two lines there is very
> little code that runs (only newproc1, if I read correctly)...

Doesn't whole init runs between these two lines? Init can create goroutines and
can do syscalls.

[remyoudompheng, 2012-03-05 18:47:20]

On 2012/03/05 11:22:04, dvyukov wrote:
> Doesn't whole init runs between these two lines? Init can create goroutines
and
> can do syscalls.

No it runs a few lines below. The goal is to set gomaxprocs>1 only after the
initial goroutine has called LockOSThread, so that it stays on the main thread.
After that all other goroutines launched by init()/runtime are free to travel
between OS threads.
Does that look OK?

http://codereview.appspot.com/5732057/diff/1003/src/pkg/runtime/proc.c
File src/pkg/runtime/proc.c (right):

http://codereview.appspot.com/5732057/diff/1003/src/pkg/runtime/proc.c#newcod...
src/pkg/runtime/proc.c:234: main·init();
init() runs here.

[rsc, 2012-03-05 21:25:33]

LGTM

Dmitriy's objection is not important here, because there are no
goroutines being created yet.

[rsc, 2012-03-05 21:40:30]

Submitting with a tweak to run.bash to test.

[rsc, 2012-03-05 21:40:39]

*** Submitted as http://code.google.com/p/go/source/detail?r=93f53e832c0f ***

runtime: wait for main goroutine before setting GOMAXPROCS.

Fixes issue 3182.

R=golang-dev, dvyukov, rsc
CC=golang-dev, remy
http://codereview.appspot.com/5732057

Committer: Russ Cox <rsc@golang.org>