io: Copy() unnecessarily allocates memory when src is a file and is empty

[daulet]

What version of Go are you using (go version)?

$ go version
go version go1.18 linux/amd64

Does this issue reproduce with the latest release?

Ye.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/dake/.cache/go-build"
GOENV="/home/dake/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/dake/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/dake/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/dake/dev/goroot"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/dake/dev/goroot/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/dake/dev/copybug/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build544396302=/tmp/go-build -gno-record-gcc-switches"

What did you do?

This affects io.Copy() when src is a file, and dst is a *net.TCPConn.

Here is a minimal benchmark: https://go.dev/play/p/zgZwpjUatSq
Benchmark with go test -bench=. -benchmem

Summary is that a gorotoutine writes to a file, another goroutine reads from the file and writes it to a TCPConn with io.Copy(). io.Copy tries to be efficient and will try to use sendfile(2), however it doesn't always succeed and ends up allocating a buffer anyway. In my deubgging it turns out the current logic believes sendfile(2) failed if we written 0 bytes. Using benchmark above one can see that we allocate 5KB/op, where op is writing a single byte.

This also affects io.CopyBuffer(), when we fail to use sendfile(2) (in case of empty src) it falls back to io.Copy(writerOnly{w}, r) which ignores passed in buffer and allocates its own.

What did you expect to see?

CopyBuffer always uses passed in buffer, Copy does not allocate extra buffer when src is empty.

What did you see instead?

Running the benchmark above shows 5GB was allocated to transfer 0.5MB worth of data, due to allocating a buffer (see size := 32 * 1024 below) when it wasn't necessary (sendfile(2) tried to copy, but there was nothing to copy).

$ go test -bench=. -benchmem -run=^\$ -memprofile memprofile.out 
goos: linux
goarch: amd64
pkg: copybug
cpu: AMD Ryzen 9 5900HS with Radeon Graphics        
BenchmarkCopy-16          549627              2187 ns/op            5391 B/op          1 allocs/op
PASS
ok      copybug 1.510s
$ go tool pprof memprofile.out
File: copybug.test
Type: alloc_space
Time: Jul 1, 2022 at 5:01pm (PDT)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 5042.76MB, 99.65% of 5060.27MB total
Dropped 25 nodes (cum <= 25.30MB)
      flat  flat%   sum%        cum   cum%
 5039.76MB 99.59% 99.59%  5056.26MB 99.92%  io.copyBuffer
       3MB 0.059% 99.65%  5042.76MB 99.65%  net.genericReadFrom
         0     0% 99.65%  5056.26MB 99.92%  copybug.BenchmarkCopy.func2
         0     0% 99.65%  5056.26MB 99.92%  io.Copy (inline)
         0     0% 99.65%  5056.26MB 99.92%  net.(*TCPConn).ReadFrom
         0     0% 99.65%  5056.26MB 99.92%  net.(*TCPConn).readFrom
(pprof) list copyBuffer
Total: 4.94GB
ROUTINE ======================== io.copyBuffer in /home/dake/dev/goroot/src/io/io.go
    4.92GB     9.86GB (flat, cum) 199.52% of Total
         .          .    407:   if wt, ok := src.(WriterTo); ok {
         .          .    408:           return wt.WriteTo(dst)
         .          .    409:   }
         .          .    410:   // Similarly, if the writer has a ReadFrom method, use it to do the copy.
         .          .    411:   if rt, ok := dst.(ReaderFrom); ok {
         .     4.94GB    412:           return rt.ReadFrom(src)
         .          .    413:   }
         .          .    414:   if buf == nil {
         .          .    415:           size := 32 * 1024
         .          .    416:           if l, ok := src.(*LimitedReader); ok && int64(size) > l.N {
         .          .    417:                   if l.N < 1 {
         .          .    418:                           size = 1
         .          .    419:                   } else {
         .          .    420:                           size = int(l.N)
         .          .    421:                   }
         .          .    422:           }
    4.92GB     4.92GB    423:           buf = make([]byte, size)
         .          .    424:   }
         .          .    425:   for {
         .          .    426:           nr, er := src.Read(buf)
         .          .    427:           if nr > 0 {
         .          .    428:                   nw, ew := dst.Write(buf[0:nr])
(pprof) 

[gopherbot]

Change https://go.dev/cl/415834 mentions this issue: io: correctly process result of sendfile(2) when src returns 0 bytes

[heschi]

cc @griesemer

[gopherbot]

Change https://go.dev/cl/536455 mentions this issue: net,internal/poll: mark it as handled even if sendfile(2) succeeded with 0 bytes sent