Commit e55fdff2 authored by Nigel Tao's avatar Nigel Tao

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
parent 2a4818dd
...@@ -569,11 +569,10 @@ func (b *Writer) WriteString(s string) (int, error) { ...@@ -569,11 +569,10 @@ func (b *Writer) WriteString(s string) (int, error) {
// ReadFrom implements io.ReaderFrom. // ReadFrom implements io.ReaderFrom.
func (b *Writer) ReadFrom(r io.Reader) (n int64, err error) { func (b *Writer) ReadFrom(r io.Reader) (n int64, err error) {
if err = b.Flush(); err != nil { if b.Buffered() == 0 {
return 0, err if w, ok := b.wr.(io.ReaderFrom); ok {
} return w.ReadFrom(r)
if w, ok := b.wr.(io.ReaderFrom); ok { }
return w.ReadFrom(r)
} }
var m int var m int
for { for {
...@@ -583,8 +582,10 @@ func (b *Writer) ReadFrom(r io.Reader) (n int64, err error) { ...@@ -583,8 +582,10 @@ func (b *Writer) ReadFrom(r io.Reader) (n int64, err error) {
} }
b.n += m b.n += m
n += int64(m) n += int64(m)
if err1 := b.Flush(); err1 != nil { if b.Available() == 0 {
return n, err1 if err1 := b.Flush(); err1 != nil {
return n, err1
}
} }
if err != nil { if err != nil {
break break
......
...@@ -881,6 +881,64 @@ func TestWriterReadFromErrors(t *testing.T) { ...@@ -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. // An onlyReader only implements io.Reader, no matter what other methods the underlying implementation may have.
type onlyReader struct { type onlyReader struct {
r io.Reader r io.Reader
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment