code review 9462044: io: Prioritize WriterTos over ReaderFroms in Copy.

io: Prioritize WriterTos over ReaderFroms in Copy.

This only affects calls where both ReaderFrom and WriterTo are implemented. WriterTo can issue one large write, while ReaderFrom must Read until EOF, potentially reallocating when out of memory. With one large Write, the Writer only needs to allocate once.

This also helps in ioutil.Discard since we can avoid copying memory when the Reader implements WriterTo.

[DMorsing, 2013-05-20 20:24:21]

Hello golang-dev@googlegroups.com (cc: bradfitz@golang.org, minux.ma@gmail.com),

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

[dsymonds, 2013-05-20 20:25:36]

Test please.

[remyoudompheng, 2013-05-20 20:37:36]

https://codereview.appspot.com/9462044/diff/4001/src/pkg/io/io.go
File src/pkg/io/io.go (right):

https://codereview.appspot.com/9462044/diff/4001/src/pkg/io/io.go#newcode334
src/pkg/io/io.go:334: // Otherwise, if src implements the WriterTo interface,
the documentation is now incorrect.

[DMorsing, 2013-05-20 21:01:43]

PTAL.

The documentation change is making me wonder if I can even do this without
breaking some sort of go1 guarantee.

https://codereview.appspot.com/9462044/diff/4001/src/pkg/io/io.go
File src/pkg/io/io.go (right):

https://codereview.appspot.com/9462044/diff/4001/src/pkg/io/io.go#newcode334
src/pkg/io/io.go:334: // Otherwise, if src implements the WriterTo interface,
On 2013/05/20 20:37:36, remyoudompheng wrote:
> the documentation is now incorrect.

Done.

[remyoudompheng, 2013-05-20 21:10:52]

On 2013/05/20 21:01:43, DMorsing wrote:
> PTAL.
> 
> The documentation change is making me wonder if I can even do this without
> breaking some sort of go1 guarantee.

I think the ReaderFrom's should check whether their Reader is a WriteTo to
reverse themselves if needed. They know better than us what to do.

Can you give an example where this would have been useful? People can reverse
ways themselves manually for optimisation whenever needed.

[DMorsing, 2013-05-21 06:55:05]

On 2013/05/20 21:10:52, remyoudompheng wrote:
> On 2013/05/20 21:01:43, DMorsing wrote:
> > PTAL.
> > 
> > The documentation change is making me wonder if I can even do this without
> > breaking some sort of go1 guarantee.
> 
> I think the ReaderFrom's should check whether their Reader is a WriteTo to
> reverse themselves if needed. They know better than us what to do.
>

The only reason for implementing WriterTo or ReaderFrom is so that you can use
your own strategy for memory allocations. Implementations shouldn't have to
judge whether their allocation strategy is less optimal than whatever is on the
other side.

> Can you give an example where this would have been useful? People can reverse
> ways themselves manually for optimisation whenever needed.

This is purely for optimization, but it is an optimization that can be
generalized.

Examples:
bytes.Buffer writing into a ioutil.Discard. Choosing the Buffer over Discard for
the From/To implementation means that we're not copying data into a black hole
buffer.

Also bytes.Buffer writing into another bytes.Buffer. If the ReadFrom is chosen,
the Writer will fill up its own buffer, allocating each time it runs out of
memory. If we choose WriteTo, we Write once and only need to do one allocation.

There's some more context for this change in
https://codereview.appspot.com/9492044/

[bradfitz, 2013-05-21 15:20:47]

LGTM

I've been trying to think of compatibility problems this could introduce, but I
haven't been able to.

We can always revert this if we think of something.

[DMorsing, 2013-05-23 16:29:27]

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

io: Prioritize WriterTos over ReaderFroms in Copy.

This only affects calls where both ReaderFrom and WriterTo are implemented.
WriterTo can issue one large write, while ReaderFrom must Read until EOF,
potentially reallocating when out of memory. With one large Write, the Writer
only needs to allocate once.

This also helps in ioutil.Discard since we can avoid copying memory when the
Reader implements WriterTo.

R=golang-dev, dsymonds, remyoudompheng, bradfitz
CC=golang-dev, minux.ma
https://codereview.appspot.com/9462044