]> Cypherpunks repositories - gostls13.git/commitdiff
archive/zip: improve BenchmarkCompressedZipGarbage
authorJosh Bleecher Snyder <josharian@gmail.com>
Thu, 28 Apr 2016 22:01:41 +0000 (15:01 -0700)
committerJosh Bleecher Snyder <josharian@gmail.com>
Sun, 1 May 2016 04:42:00 +0000 (04:42 +0000)
Before this CL:

$ go test -bench=CompressedZipGarbage -count=5 -run=NONE archive/zip
BenchmarkCompressedZipGarbage-8        50  20677087 ns/op   42973 B/op      47 allocs/op
BenchmarkCompressedZipGarbage-8       100  20584764 ns/op   24294 B/op      47 allocs/op
BenchmarkCompressedZipGarbage-8        50  20859221 ns/op   42973 B/op      47 allocs/op
BenchmarkCompressedZipGarbage-8       100  20901176 ns/op   24294 B/op      47 allocs/op
BenchmarkCompressedZipGarbage-8        50  21282409 ns/op   42973 B/op      47 allocs/op

The B/op number is effectively meaningless. There
is a surprisingly large one-time cost that gets
divided by the number of iterations that your
machine can get through in a second.

This CL discards the first run, which helps.
It is not a panacea. Running with -benchtime=10s
will allow the sync.Pool to be emptied,
which brings the problem back.
However, since there are more iterations to divide
the cost through, it’s not quite as bad,
and running with a high benchtime is rare.

This CL changes the meaning of the B/op number,
which is unfortunate, since it won’t have the
same order of magnitude as previous Go versions.
But it wasn’t really comparable before anyway,
since it didn’t have any reliable meaning at all.

After this CL:

$ go test -bench=CompressedZipGarbage -count=5 -run=NONE archive/zip
BenchmarkCompressedZipGarbage-8         100   20881890 ns/op     5616 B/op       47 allocs/op
BenchmarkCompressedZipGarbage-8          50   20622757 ns/op     5616 B/op       47 allocs/op
BenchmarkCompressedZipGarbage-8          50   20628193 ns/op     5616 B/op       47 allocs/op
BenchmarkCompressedZipGarbage-8         100   20756612 ns/op     5616 B/op       47 allocs/op
BenchmarkCompressedZipGarbage-8         100   20639774 ns/op     5616 B/op       47 allocs/op

Change-Id: Iedee04f39328974c7fa272a6113d423e7ffce50f
Reviewed-on: https://go-review.googlesource.com/22585
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
src/archive/zip/writer_test.go

index 01b63f2358d41c258baf08b1f5206b11f41dfcc9..86841c755f160ee371976556d03b1506b7c271e2 100644 (file)
@@ -184,7 +184,7 @@ func BenchmarkCompressedZipGarbage(b *testing.B) {
        b.ReportAllocs()
        var buf bytes.Buffer
        bigBuf := bytes.Repeat([]byte("a"), 1<<20)
-       for i := 0; i < b.N; i++ {
+       for i := 0; i <= b.N; i++ {
                buf.Reset()
                zw := NewWriter(&buf)
                for j := 0; j < 3; j++ {
@@ -195,5 +195,11 @@ func BenchmarkCompressedZipGarbage(b *testing.B) {
                        w.Write(bigBuf)
                }
                zw.Close()
+               if i == 0 {
+                       // Reset the timer after the first time through.
+                       // This effectively discards the very large initial flate setup cost,
+                       // as well as the initialization of bigBuf.
+                       b.ResetTimer()
+               }
        }
 }