code review 51360046: bufio: fix benchmarks behavior

bufio: fix benchmarks behavior
Currently the benchmarks lie to testing package by doing O(N)
work under StopTimer. And that hidden O(N) actually consitutes
the bulk of benchmark work (e.g includes GC per iteration).
This behavior accounts for windows-amd64-race builder hangs.

Before:
BenchmarkReaderCopyOptimal-4	 1000000	      1861 ns/op
BenchmarkReaderCopyUnoptimal-4	  500000	      3327 ns/op
BenchmarkReaderCopyNoWriteTo-4	   50000	     34549 ns/op
BenchmarkWriterCopyOptimal-4	  100000	     16849 ns/op
BenchmarkWriterCopyUnoptimal-4	  500000	      3126 ns/op
BenchmarkWriterCopyNoReadFrom-4	   50000	     34609 ns/op
ok  	bufio	65.273s

After:
BenchmarkReaderCopyOptimal-4	10000000	       172 ns/op
BenchmarkReaderCopyUnoptimal-4	10000000	       267 ns/op
BenchmarkReaderCopyNoWriteTo-4	  100000	     22905 ns/op
BenchmarkWriterCopyOptimal-4	10000000	       170 ns/op
BenchmarkWriterCopyUnoptimal-4	10000000	       226 ns/op
BenchmarkWriterCopyNoReadFrom-4	  100000	     20575 ns/op
ok  	bufio	14.074s

Note the change in total time.

[dvyukov, 2014-01-23 10:08:42]

Hello golang-codereviews@googlegroups.com (cc: alex.brainman@gmail.com),

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

[brainman, 2014-01-23 12:22:52]

LGTM

but I don't know enough to test it properly. Perhaps you should wait for others.

I could not test it on windows either. I am traveling and don't have computer.

Alex

https://codereview.appspot.com/51360046/diff/10002/src/pkg/bufio/bufio_test.go
File src/pkg/bufio/bufio_test.go (right):

https://codereview.appspot.com/51360046/diff/10002/src/pkg/bufio/bufio_test.g...
src/pkg/bufio/bufio_test.go:1186: dstReader := NewWriter(dstBuf)
Perhaps I don't understand your code, but this should be called dstWriter, not
dstReader.

[dvyukov, 2014-01-23 14:56:12]

thanks!

https://codereview.appspot.com/51360046/diff/10002/src/pkg/bufio/bufio_test.go
File src/pkg/bufio/bufio_test.go (right):

https://codereview.appspot.com/51360046/diff/10002/src/pkg/bufio/bufio_test.g...
src/pkg/bufio/bufio_test.go:1186: dstReader := NewWriter(dstBuf)
On 2014/01/23 12:22:53, brainman wrote:
> Perhaps I don't understand your code, but this should be called dstWriter, not
> dstReader.

Done.

[rsc, 2014-01-23 20:13:06]

LGTM

[rsc, 2014-01-23 20:13:25]

*** Submitted as https://code.google.com/p/go/source/detail?r=847f3d4e7104 ***

bufio: fix benchmarks behavior
Currently the benchmarks lie to testing package by doing O(N)
work under StopTimer. And that hidden O(N) actually consitutes
the bulk of benchmark work (e.g includes GC per iteration).
This behavior accounts for windows-amd64-race builder hangs.

Before:
BenchmarkReaderCopyOptimal-4	 1000000	      1861 ns/op
BenchmarkReaderCopyUnoptimal-4	  500000	      3327 ns/op
BenchmarkReaderCopyNoWriteTo-4	   50000	     34549 ns/op
BenchmarkWriterCopyOptimal-4	  100000	     16849 ns/op
BenchmarkWriterCopyUnoptimal-4	  500000	      3126 ns/op
BenchmarkWriterCopyNoReadFrom-4	   50000	     34609 ns/op
ok  	bufio	65.273s

After:
BenchmarkReaderCopyOptimal-4	10000000	       172 ns/op
BenchmarkReaderCopyUnoptimal-4	10000000	       267 ns/op
BenchmarkReaderCopyNoWriteTo-4	  100000	     22905 ns/op
BenchmarkWriterCopyOptimal-4	10000000	       170 ns/op
BenchmarkWriterCopyUnoptimal-4	10000000	       226 ns/op
BenchmarkWriterCopyNoReadFrom-4	  100000	     20575 ns/op
ok  	bufio	14.074s

Note the change in total time.

LGTM=alex.brainman, rsc
R=golang-codereviews, alex.brainman, rsc
CC=golang-codereviews
https://codereview.appspot.com/51360046

Committer: Russ Cox <rsc@golang.org>