io: consider reusing buffers for io.Copy to reduce GC pressure

[artyom]

I wonder whether the possibility of pooling (reusing) buffers was considered for io package. io.Copy is used in standard library internals (i.e. net/http), and some workloads can benefit from buffer reusage inside io package, which reduces GC pressure.

Consider the following patch:

diff --git i/src/io/io.go w/src/io/io.go
index 8851eaf..accbb7f 100644
--- i/src/io/io.go
+++ w/src/io/io.go
@@ -14,6 +14,7 @@ package io

 import (
    "errors"
+   "sync"
 )

 // ErrShortWrite means that a write accepted fewer bytes than requested
@@ -375,7 +376,8 @@ func copyBuffer(dst Writer, src Reader, buf []byte) (written int64, err error) {
        return rt.ReadFrom(src)
    }
    if buf == nil {
-       buf = make([]byte, 32*1024)
+       buf = bufPool.Get().([]byte)
+       defer bufPool.Put(buf)
    }
    for {
        nr, er := src.Read(buf)
@@ -519,3 +521,7 @@ func (t *teeReader) Read(p []byte) (n int, err error) {
    }
    return
 }
+
+var bufPool = sync.Pool{
+   New: func() interface{} { return make([]byte, 32*1024) },
+}

I have an app using net/http which receives requests at a high rate. Before applying this patch the GC triggered every ~17 seconds reporting 6% cpu usage:

gc 2203 @36423.458s 6%: 0.98+607+0.002+780+150 ms clock, 0.98+607+0+420/187/0+150 ms cpu, 673->684->426 MB, 686 MB goal, 1 P
gc 2204 @36440.582s 6%: 0.84+1388+0.003+2324+250 ms clock, 0.84+1388+0+1208/560/0+250 ms cpu, 672->689->420 MB, 686 MB goal, 1 

after applying the patch above, the same app running under the same workload sees significantly less frequent GC runs: >90 seconds intervals and cpu usage reported at about 1-2%:

gc 35 @2085.673s 1%: 1.1+601+0.002+1716+129 ms clock, 1.1+601+0+144/440/339+129 ms cpu, 593->599->382 MB, 605 MB goal, 1 P
gc 36 @2182.304s 1%: 1.1+576+0.004+951+89 ms clock, 1.1+576+0+501/244/0+89 ms cpu, 609->615->374 MB, 621 MB goal, 1 P

[crawshaw]

Go 1.5 introduced io.CopyBuffer to let you reuse buffers.

@bradfitz probably remembers why sync.Pool didn't work here.

[artyom]

@crawshaw io.Copy is used at least in net/http internals — I'm mostly interested in cases where one cannot replace io.Copy with io.CopyBuffer without forking part of stdlib (or building from modified goroot).

See for example:

go (master) $ find src/net/http -name \*.go -type f ! -name \*_test.go -exec cat '{}' \+ | grep -wc io.Copy
19

at least some of those cases falls to io.copyBuffer(dst, src, nil) when neither io.WriterTo nor io.ReaderFrom optimizations apply (which allocates non-reusable []byte per function call).

[crawshaw]

It looks like at least some of those uses of io.Copy in net/http could be replaced by io.CopyBuffer, reusing a []byte held by the objects in http. Want to give it a shot?

[artyom]

I can work on replacing some of io.Copy calls with io.CopyBuffer in net/http, but would like to have @bradfitz to comment on this first.

I think now that changing net/http would make more sense than modifying io.Copy, as net/http is the most heavy user of io.Copy throughout stdlib.

[bradfitz]

See old discussion in #5509 and the patches it references.

[crawshaw]

@bradfitz that covers the first part of this, thanks. What about the later question, replacing some uses of io.Copy in net/http with io.CopyBuffer?

[bradfitz]

Closing this bug as a dup of #5509. Please open a separate bug (or just send CLs) for net/http performance improvements, which I can review.