code review 4306043: runtime: cpu profiling support

runtime: cpu profiling support

[rsc, 2011-03-22 20:42:53]

Hello r (cc: golang-dev@googlegroups.com),

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

[rsc, 2011-03-22 20:44:57]

In this CL the most interesting file is cpuprof.c.

The changes to the many signal.c are formulaic
and all the same spirit, if not the same code,
thanks to per-OS or per-arch differences.

Russ

[r, 2011-03-22 21:10:35]

LGTM

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/cpuprof.c
File src/pkg/runtime/cpuprof.c (right):

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/cpuprof.c#ne...
src/pkg/runtime/cpuprof.c:47: // handoff using atomic operations.  The atomics
are needed, however,
s/The atomics/Those operations

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/cpuprof.c#ne...
src/pkg/runtime/cpuprof.c:113: // discarded due to slow data writers.
nice

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/cpuprof.c#ne...
src/pkg/runtime/cpuprof.c:151: *p++ = 0;  // version number
cpprof version header?
maybe reference the docs here - it's pure magic

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/cpuprof.c#ne...
src/pkg/runtime/cpuprof.c:166: runtime·printf("profiler off count=%d
totallost=%d\n", (int32)prof->count, (int32)prof->totallost);
d?

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/debug.go
File src/pkg/runtime/debug.go (right):

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/debug.go#new...
src/pkg/runtime/debug.go:96: // of calling MemProfile directly.
maybe mention gotest

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/debug.go#new...
src/pkg/runtime/debug.go:101: // blocks until profiling data is available. 
Otherwise CPUProfile returns nil.
that 'otherwise' is confusing. rewrite by regrouping i think

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/debug.go#new...
src/pkg/runtime/debug.go:104: // CPUProfile directly.
gotest as a TODO?

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/debug.go#new...
src/pkg/runtime/debug.go:108: // If hz == 0, SetCPUProfileRate turns off
profiling.
maybe <= 0 is better

http://codereview.appspot.com/4306043/diff/15001/src/pkg/runtime/debug.go#new...
src/pkg/runtime/debug.go:111: // SetCPUProfileRate directly.
gotest again?

[rsc, 2011-03-23 15:43:36]

Hello r (cc: golang-dev@googlegroups.com),

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

[rsc, 2011-03-23 15:43:44]

*** Submitted as 741cebe073f1 ***

runtime: cpu profiling support

R=r
CC=golang-dev
http://codereview.appspot.com/4306043