From e55fdff21030e4925086af90fa669b3e378f2dfc Mon Sep 17 00:00:00 2001
From: Nigel Tao <nigeltao@golang.org>
Date: Fri, 19 Oct 2012 16:32:00 +1100
Subject: [PATCH] bufio: make Writer.ReadFrom not flush prematurely. For
 example, many small writes to a network may be less efficient that a few
 large writes.

This fixes net/http's TestClientWrites, broken by 6565056 that
introduced Writer.ReadFrom. That test implicitly assumed that
calling io.Copy on a *bufio.Writer wouldn't write to the
underlying network until the buffer was full.

R=dsymonds
CC=bradfitz, golang-dev, mchaten, mikioh.mikioh
https://golang.org/cl/6743044
---
 src/pkg/bufio/bufio.go      | 15 +++++-----
 src/pkg/bufio/bufio_test.go | 58 +++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/src/pkg/bufio/bufio.go b/src/pkg/bufio/bufio.go
index d1c5a13bca..cd51585f84 100644
--- a/src/pkg/bufio/bufio.go
+++ b/src/pkg/bufio/bufio.go
@@ -569,11 +569,10 @@ func (b *Writer) WriteString(s string) (int, error) {
 
 // ReadFrom implements io.ReaderFrom.
 func (b *Writer) ReadFrom(r io.Reader) (n int64, err error) {
-	if err = b.Flush(); err != nil {
-		return 0, err
-	}
-	if w, ok := b.wr.(io.ReaderFrom); ok {
-		return w.ReadFrom(r)
+	if b.Buffered() == 0 {
+		if w, ok := b.wr.(io.ReaderFrom); ok {
+			return w.ReadFrom(r)
+		}
 	}
 	var m int
 	for {
@@ -583,8 +582,10 @@ func (b *Writer) ReadFrom(r io.Reader) (n int64, err error) {
 		}
 		b.n += m
 		n += int64(m)
-		if err1 := b.Flush(); err1 != nil {
-			return n, err1
+		if b.Available() == 0 {
+			if err1 := b.Flush(); err1 != nil {
+				return n, err1
+			}
 		}
 		if err != nil {
 			break
diff --git a/src/pkg/bufio/bufio_test.go b/src/pkg/bufio/bufio_test.go
index 3d07639e2a..763b12326a 100644
--- a/src/pkg/bufio/bufio_test.go
+++ b/src/pkg/bufio/bufio_test.go
@@ -881,6 +881,64 @@ func TestWriterReadFromErrors(t *testing.T) {
 	}
 }
 
+// TestWriterReadFromCounts tests that using io.Copy to copy into a
+// bufio.Writer does not prematurely flush the buffer. For example, when
+// buffering writes to a network socket, excessive network writes should be
+// avoided.
+func TestWriterReadFromCounts(t *testing.T) {
+	var w0 writeCountingDiscard
+	b0 := NewWriterSize(&w0, 1234)
+	b0.WriteString(strings.Repeat("x", 1000))
+	if w0 != 0 {
+		t.Fatalf("write 1000 'x's: got %d writes, want 0", w0)
+	}
+	b0.WriteString(strings.Repeat("x", 200))
+	if w0 != 0 {
+		t.Fatalf("write 1200 'x's: got %d writes, want 0", w0)
+	}
+	io.Copy(b0, &onlyReader{strings.NewReader(strings.Repeat("x", 30))})
+	if w0 != 0 {
+		t.Fatalf("write 1230 'x's: got %d writes, want 0", w0)
+	}
+	io.Copy(b0, &onlyReader{strings.NewReader(strings.Repeat("x", 9))})
+	if w0 != 1 {
+		t.Fatalf("write 1239 'x's: got %d writes, want 1", w0)
+	}
+
+	var w1 writeCountingDiscard
+	b1 := NewWriterSize(&w1, 1234)
+	b1.WriteString(strings.Repeat("x", 1200))
+	b1.Flush()
+	if w1 != 1 {
+		t.Fatalf("flush 1200 'x's: got %d writes, want 1", w1)
+	}
+	b1.WriteString(strings.Repeat("x", 89))
+	if w1 != 1 {
+		t.Fatalf("write 1200 + 89 'x's: got %d writes, want 1", w1)
+	}
+	io.Copy(b1, &onlyReader{strings.NewReader(strings.Repeat("x", 700))})
+	if w1 != 1 {
+		t.Fatalf("write 1200 + 789 'x's: got %d writes, want 1", w1)
+	}
+	io.Copy(b1, &onlyReader{strings.NewReader(strings.Repeat("x", 600))})
+	if w1 != 2 {
+		t.Fatalf("write 1200 + 1389 'x's: got %d writes, want 2", w1)
+	}
+	b1.Flush()
+	if w1 != 3 {
+		t.Fatalf("flush 1200 + 1389 'x's: got %d writes, want 3", w1)
+	}
+}
+
+// A writeCountingDiscard is like ioutil.Discard and counts the number of times
+// Write is called on it.
+type writeCountingDiscard int
+
+func (w *writeCountingDiscard) Write(p []byte) (int, error) {
+	*w++
+	return len(p), nil
+}
+
 // An onlyReader only implements io.Reader, no matter what other methods the underlying implementation may have.
 type onlyReader struct {
 	r io.Reader
-- 
2.48.1