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://golang.org/cl/9462044

diff --git a/src/pkg/io/io.go b/src/pkg/io/io.go
index ec2cd6056fe66a0b5fc7bce18f150cfb3b07a816..16c825fdbc1086622ab284c029d7809ab90350da 100644 (file)
--- a/src/pkg/io/io.go
+++ b/src/pkg/io/io.go
@@ -329,20 +329,20 @@ func CopyN(dst Writer, src Reader, n int64) (written int64, err error) {
 // Because Copy is defined to read from src until EOF, it does
 // not treat an EOF from Read as an error to be reported.
 //
-// If dst implements the ReaderFrom interface,
-// the copy is implemented by calling dst.ReadFrom(src).
-// Otherwise, if src implements the WriterTo interface,
+// If src implements the WriterTo interface,
 // the copy is implemented by calling src.WriteTo(dst).
+// Otherwise, if dst implements the ReaderFrom interface,
+// the copy is implemented by calling dst.ReadFrom(src).
 func Copy(dst Writer, src Reader) (written int64, err error) {
-       // If the writer has a ReadFrom method, use it to do the copy.
+       // If the reader has a WriteTo method, use it to do the copy.
        // Avoids an allocation and a copy.
-       if rt, ok := dst.(ReaderFrom); ok {
-               return rt.ReadFrom(src)
-       }
-       // Similarly, if the reader has a WriteTo method, use it to do the copy.
        if wt, ok := src.(WriterTo); ok {
                return wt.WriteTo(dst)
        }
+       // Similarly, if the writer has a ReadFrom method, use it to do the copy.
+       if rt, ok := dst.(ReaderFrom); ok {
+               return rt.ReadFrom(src)
+       }
        buf := make([]byte, 32*1024)
        for {
                nr, er := src.Read(buf)

diff --git a/src/pkg/io/io_test.go b/src/pkg/io/io_test.go
index 1bc451e444a01f1a0f4e89a832310134fd2b7a30..dc7df0288e60c9ef8f8a79930f899504ae328cff 100644 (file)
--- a/src/pkg/io/io_test.go
+++ b/src/pkg/io/io_test.go
@@ -52,6 +52,32 @@ func TestCopyWriteTo(t *testing.T) {
        }
 }
 
+// Version of bytes.Buffer that checks whether WriteTo was called or not
+type writeToChecker struct {
+       bytes.Buffer
+       writeToCalled bool
+}
+
+func (wt *writeToChecker) WriteTo(w Writer) (int64, error) {
+       wt.writeToCalled = true
+       return wt.Buffer.WriteTo(w)
+}
+
+// It's preferable to choose WriterTo over ReaderFrom, since a WriterTo can issue one large write,
+// while the ReaderFrom must read until EOF, potentially allocating when running out of buffer.
+// Make sure that we choose WriterTo when both are implemented.
+func TestCopyPriority(t *testing.T) {
+       rb := new(writeToChecker)
+       wb := new(bytes.Buffer)
+       rb.WriteString("hello, world.")
+       Copy(wb, rb)
+       if wb.String() != "hello, world." {
+               t.Errorf("Copy did not work properly")
+       } else if !rb.writeToCalled {
+               t.Errorf("WriteTo was not prioritized over ReadFrom")
+       }
+}
+
 func TestCopyN(t *testing.T) {
        rb := new(Buffer)
        wb := new(Buffer)