code review 6587069: benchcmp: show memory statistics, when available

benchcmp: show memory statistics, when available

[jeff.allen, 2012-10-03 12:24:54]

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

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

[minux1, 2012-10-03 15:10:41]

does "MB/s" always there when there is "allocs/op"?

[jeff.allen, 2012-10-03 15:26:28]

On 2012/10/03 15:10:41, minux wrote:
> does "MB/s" always there when there is "allocs/op"?

Sorry, don't understand the question. Both MB/s and allocs/op appear if and only
if both the old and new benchmark results have that measurement in them.

[minux1, 2012-10-03 15:30:17]

On Wednesday, October 3, 2012, wrote:

> On 2012/10/03 15:10:41, minux wrote:
>
>> does "MB/s" always there when there is "allocs/op"?
>
> Sorry, don't understand the question. Both MB/s and allocs/op appear if
> and only if both the old and new benchmark results have that measurement
>
sorry i didn't make myself clear.
what if a benchmark shows allocs/op but not MB/s?

[jeff.allen, 2012-10-16 20:39:01]

With this change, benchcmp compares that which is available to compare. If there
are allocs/op but not MB/s, it compares only allocs/op.

[minux1, 2012-10-22 16:06:32]

https://codereview.appspot.com/6587069/diff/4001/misc/benchcmp
File misc/benchcmp (right):

https://codereview.appspot.com/6587069/diff/4001/misc/benchcmp#newcode30
misc/benchcmp:30: if($8 == "allocs/op")
the problem is:
you hardcode column number for "allocs/op" here, which might
not be correct.

in fact, go test -benchmem -bench=. crypto/aes gives this
(slightly edited the spaces):
BenchmarkEncrypt 50000000 37.7 ns/op  424.75 MB/s  0 B/op  0 allocs/op
BenchmarkDecrypt 50000000 38.4 ns/op  416.28 MB/s  0 B/op  0 allocs/op
BenchmarkExpand  50000000 66.9 ns/op   0 B/op  0 allocs/op
ok  	crypto/aes	7.360s

On the first two lines, allocs/op is at the 10th column, but
on the third line it is at the 8th column.

[jeff.allen, 2012-10-23 10:16:08]

OK, good catch. Fixed it by trying both positions, hacky but seems reasonable
for this little testing tool. I also fixed an old typo in benchcmp (= versus
==). Finally, corrected divide by zero to not crash.

Please take another look.

[jeff.allen, 2012-10-30 16:03:37]

Fixed formatting of allocations (no longer displayed as floats).

PTAL

[dave_cheney.net, 2012-10-30 16:09:41]

Is it possible to have the delta displayed in the same format as the ns/op
section for function duration ?

[jeff.allen, 2012-10-31 14:09:14]

Done. Example of the current format (ignore the data, it's false):


benchmark                old ns/op    new ns/op    delta
BenchmarkClientServer       321861       354859  +10.25%

benchmark               old allocs   new allocs    delta
BenchmarkClientServer          121          116   -4.13%

[dave_cheney.net, 2012-10-31 14:11:17]

Thank you, it's good to be consistent.


On Thu, Nov 1, 2012 at 1:09 AM,  <jeff.allen@gmail.com> wrote:
> Done. Example of the current format (ignore the data, it's false):
>
>
> benchmark                old ns/op    new ns/op    delta
> BenchmarkClientServer       321861       354859  +10.25%
>
> benchmark               old allocs   new allocs    delta
> BenchmarkClientServer          121          116   -4.13%
>
>
> https://codereview.appspot.com/6587069/

[extra, 2012-10-31 18:30:14]

On 2012/10/31 14:09:14, jeff.allen wrote:
> Done. Example of the current format (ignore the data, it's false):
> 
> 
> benchmark                old ns/op    new ns/op    delta
> BenchmarkClientServer       321861       354859  +10.25%
> 
> benchmark               old allocs   new allocs    delta
> BenchmarkClientServer          121          116   -4.13%

Update the usage?

benchcmp -h
usage: benchcmp old.txt new.txt

Each input file should be go test -test.run=NONE -test.bench=. > [old,new].txt
Benchcmp compares the first and last for each benchmark.

[rsc, 2012-11-01 16:35:20]

looks fine; leaving for minux to do the final lgtm and submit
thanks!

https://codereview.appspot.com/6587069/diff/20002/misc/benchcmp
File misc/benchcmp (right):

https://codereview.appspot.com/6587069/diff/20002/misc/benchcmp#newcode34
misc/benchcmp:34: newall[$1] = $7
all sounds all and not a shortening of alloc. please use newalloc and oldalloc.

https://codereview.appspot.com/6587069/diff/20002/misc/benchcmp#newcode78
misc/benchcmp:78: anyall = 0
anyalloc

[jeff.allen, 2012-11-02 15:58:51]

Hello minux.ma@gmail.com, dave@cheney.net, extraterrestrial.neighbour@gmail.com,
rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[minux1, 2012-11-02 16:34:46]

LGTM. Thanks.

[minux1, 2012-11-02 18:14:04]

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

misc/benchcmp: show memory statistics, when available

R=minux.ma, dave, extraterrestrial.neighbour, rsc
CC=golang-dev
http://codereview.appspot.com/6587069

Committer: Shenghou Ma <minux.ma@gmail.com>