code review 6498060: cgo/misc/test: burn CPU to improve sleep accuracy

cgo/misc/test: burn CPU to improve sleep accuracy

Fixes issue 4008.

Run a background goroutine that wastes CPU to trick the
power management into raising the CPU frequency which,
by side effect, makes sleep more accurate on arm.

=== RUN TestParallelSleep
--- PASS: TestParallelSleep (1.30 seconds)
_cgo_gotypes.go:772:    sleep(1) slept for 1.000458s

[dave_cheney.net, 2012-08-30 03:50:22]

Hello minux.ma@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[minux1, 2012-08-30 09:48:26]

i think this is a better solution than CL 6488056.

the solution SGTM, but i want to hear others' comment.

http://codereview.appspot.com/6498060/diff/7001/misc/cgo/test/issue1560.go
File misc/cgo/test/issue1560.go (right):

http://codereview.appspot.com/6498060/diff/7001/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:42: func wasteCPU() chan struct{} {
do you want to limit the scope of this workaround?

http://codereview.appspot.com/6498060/diff/7001/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:62: runtime.GOMAXPROCS(2)
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2))

[dave_cheney.net, 2012-08-30 10:55:20]

Hello minux.ma@gmail.com (cc: golang-dev@googlegroups.com),

Please take another look.

[dave_cheney.net, 2012-08-30 10:56:02]

Thank you for your comments.

http://codereview.appspot.com/6498060/diff/7001/misc/cgo/test/issue1560.go
File misc/cgo/test/issue1560.go (right):

http://codereview.appspot.com/6498060/diff/7001/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:42: func wasteCPU() chan struct{} {
On 2012/08/30 09:48:26, minux wrote:
> do you want to limit the scope of this workaround?

Not quite sure what you are saying here ? Should I make this an anon func inside
testParallelSleep?

http://codereview.appspot.com/6498060/diff/7001/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:62: runtime.GOMAXPROCS(2)
On 2012/08/30 09:48:26, minux wrote:
> defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2))

Done.

[minux1, 2012-08-30 10:59:37]

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go
File misc/cgo/test/issue1560.go (right):

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:61: runtime.GOMAXPROCS(2)
how about this?
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2))

runtime.GOMAXPROCS() will always return the old setting.

[dave_cheney.net, 2012-08-30 11:01:02]

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go
File misc/cgo/test/issue1560.go (right):

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:61: runtime.GOMAXPROCS(2)
On 2012/08/30 10:59:37, minux wrote:
> how about this?
> defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2))
> 
> runtime.GOMAXPROCS() will always return the old setting.

nice. that is very compact now

func testParallelSleep(t *testing.T) {
        defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2))
        defer close(wasteCPU())

[r, 2012-08-31 00:11:34]

wow cool. or hot.

after minux's suggestion this LGTM but someone who knows arm better should
probably ok it. minux?

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go
File misc/cgo/test/issue1560.go (right):

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:39: // CPU which causes the power management to raise
the
s/which causes/to cause/

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:40: // CPU frequency. On arm this has the side effect
of
s/arm/ARM/

[dave_cheney.net, 2012-08-31 00:14:40]

Thank you for your comments, PTAL.

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go
File misc/cgo/test/issue1560.go (right):

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:39: // CPU which causes the power management to raise
the
On 2012/08/31 00:11:34, r wrote:
> s/which causes/to cause/

Done.

http://codereview.appspot.com/6498060/diff/5002/misc/cgo/test/issue1560.go#ne...
misc/cgo/test/issue1560.go:40: // CPU frequency. On arm this has the side effect
of
On 2012/08/31 00:11:34, r wrote:
> s/arm/ARM/

Done. It is sometimes unclear which version to use.

[dave_cheney.net, 2012-08-31 00:15:00]

Hello minux.ma@gmail.com, r@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[r, 2012-08-31 00:16:09]

LGTM but wait for minux

[dave_cheney.net, 2012-08-31 02:18:57]

On 2012/08/31 00:16:09, r wrote:
> LGTM but wait for minux

Here is a datapoint for trying to sleep for 1 second.

wc -l /tmp/timing.txt && sort /tmp/timing.txt | tail 
72350 /tmp/timing.txt
Elapsed 2564027
Elapsed 2564445
Elapsed 2564594
Elapsed 2564761
Elapsed 2564774
Elapsed 2565149
Elapsed 2566092
Elapsed 2567279
Elapsed 2567445
Elapsed 2568912

The worst case was 2.56 seconds.

[minux1, 2012-08-31 07:39:52]

LGTM.

Thanks.

[dave_cheney.net, 2012-08-31 10:18:13]

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

cgo/misc/test: burn CPU to improve sleep accuracy

Fixes issue 4008.

Run a background goroutine that wastes CPU to trick the
power management into raising the CPU frequency which,
by side effect, makes sleep more accurate on arm.

=== RUN TestParallelSleep
--- PASS: TestParallelSleep (1.30 seconds)
_cgo_gotypes.go:772:    sleep(1) slept for 1.000458s

R=minux.ma, r
CC=golang-dev
http://codereview.appspot.com/6498060