From fdc4ce6ec790b1a0507c3c2ef20e94aca4876a1b Mon Sep 17 00:00:00 2001 From: Daniel Morsing Date: Thu, 23 May 2013 18:29:19 +0200 Subject: [PATCH] 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 --- src/pkg/io/io.go | 16 ++++++++-------- src/pkg/io/io_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/pkg/io/io.go b/src/pkg/io/io.go index ec2cd6056f..16c825fdbc 100644 --- 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 1bc451e444..dc7df0288e 100644 --- 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) -- 2.48.1