code review 107440043: archive/tar: reuse temporary buffer in writeHeader

archive/tar: reuse temporary buffer in writeHeader

A temporary 512 bytes buffer is allocated for every call to
writeHeader. This buffer could be reused the lower the number
of memory allocations.

benchmark                   old ns/op     new ns/op     delta
BenchmarkWriteFiles100k     634622051     583810847     -8.01%

benchmark                   old allocs     new allocs     delta
BenchmarkWriteFiles100k     2701920        2602621        -3.68%

benchmark                   old bytes     new bytes     delta
BenchmarkWriteFiles100k     115383884     64349922      -44.23%

This change is very important if your code has to write a lot of
tarballs with a lot of files.

[unclejack, 2014-07-02 09:01:38]

Hello golang-codereviews@googlegroups.com, dave@cheney.net, dsymonds@golang.org
(cc: golang-codereviews@googlegroups.com),

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

[dave_cheney.net, 2014-07-02 10:11:22]

https://codereview.appspot.com/107440043/diff/40001/src/pkg/archive/tar/write...
File src/pkg/archive/tar/writer.go (right):

https://codereview.appspot.com/107440043/diff/40001/src/pkg/archive/tar/write...
src/pkg/archive/tar/writer.go:165: copy(tw.hdrBuff[:], zeroBlock)
copy(header, zeroBlock)

https://codereview.appspot.com/107440043/diff/40001/src/pkg/archive/tar/write...
src/pkg/archive/tar/writer.go:166: s := slicer(header)
why does slicer take a []byte, when we just copied this []byte from zeroBlock

[unclejack, 2014-07-02 11:58:38]

dfc,

I've updated both CLs.

https://codereview.appspot.com/107440043/diff/40001/src/pkg/archive/tar/write...
File src/pkg/archive/tar/writer.go (right):

https://codereview.appspot.com/107440043/diff/40001/src/pkg/archive/tar/write...
src/pkg/archive/tar/writer.go:165: copy(tw.hdrBuff[:], zeroBlock)
On 2014/07/02 10:11:22, dfc wrote:
> copy(header, zeroBlock)

Done.

https://codereview.appspot.com/107440043/diff/40001/src/pkg/archive/tar/write...
src/pkg/archive/tar/writer.go:166: s := slicer(header)
On 2014/07/02 10:11:22, dfc wrote:
> why does slicer take a []byte, when we just copied this []byte from zeroBlock

This is equivalent to the previous behaviour. However, it does need some
explanation.

[dsymonds, 2014-07-02 23:39:07]

LGTM

[dsymonds, 2014-07-02 23:44:27]

The codereview plugin barfed while I was submitting this. It did get submitted
(https://code.google.com/p/go/source/detail?r=837348e418f33fc7a242f56dbe2feff8...),
but this codereview thread didn't get updated.

[gobot, 2014-07-03 10:02:50]

R=close (assigned by dave@cheney.net)

[unclejack, 2014-07-08 12:02:27]

*** Abandoned ***