archive/zip: make checksumReader implement io.WriterTo

[aktau]

We've stumbled upon a case where performance is left on the table in io.CopyBuffer when copying from an archive/zip.checksumReader to an *os.File:

var dst *os.File // ...
var src *zip.checksumReader // ...
var buf [32 * 1024]byte
io.CopyBuffer(dst, src, buf[:])

In this case, profiles [1] show that buf is unused, and io.CopyBuffer ends up allocating in a child call. The allocations take up 8% of cycles in an exemplar of a fast compilation step (which uses an auxiliary Go binary featuring zip files) at our company.

We can work around this problem now that we've discovered it using the struct hack mentioned in #16474, but this leaves performance on the table for others that copy from zip files to real files.

[1]:

[ianlancetaylor]

checksumReader is not an exported type, so adding a WriteTo method will not affect the public API. This can just be an ordinary issue. Taking it out of the proposal process.

[ianlancetaylor]

Oh, I see, it is returned by (*File).Open, as an io.ReadCloser. Still, I think we can make this change, if it is desirable, outside of the proposal process.

[ianlancetaylor]

This is a specific instance of #16474.

[ianlancetaylor]

Note that implementing checksumReader.WriteTo is still going to allocate a buffer, and is not going to use the buffer passed to io.CopyBuffer. I'm not sure how that will help.

[aktau]

You're right, of course. I had not considered this properly. Even in the very best case:

1. checksumReader does streaming writes to the io.Writer, and
2. The io.Writer buffers internally (or is wrapped with a bufio.Writer)

I can't imagine this being more (if at all) efficient than io.CopyBuffer.

So perhaps the proposal should be reversed: never implement WriteTo for checksumReader because then my current optimization io.CopyBuffer(struct{io.Writer}{f}, zr, buf[:]) would be invalidated.