bufio: Reader should fill the read destination slice if possible

[paulstuart]

Please answer these questions before submitting your issue. Thanks!

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

go version go1.9.2 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/pstuart/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/qt/_9gpg7kx2zn0180w_tftkqgh0000gn/T/go-build893477021=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

Used a bufio reader to read small chunks from a large io stream

Example here: https://play.golang.org/p/ufs826X0l7

What did you expect to see?

I expected the read to fill my buffer completely

What did you see instead?

I only saw what was left in the buffer

This is fixed by adding the following at line 222 of bufio.go:

    // refill buffer if needed
    if len(p) > b.w-b.r {
            // Slide existing data to beginning.
            if b.r > 0 {
                    copy(b.buf, b.buf[b.r:b.w])
                    b.w -= b.r
                    b.r = 0
            }

            n, err := b.rd.Read(b.buf[b.w:])
            if n < 0 {
                    panic(errNegativeRead)
            }
            b.w += n
            if err != nil {
                    b.err = err
            }
    }

[as]

Just to clarify, The example reads (1) 4096 + (2) 2048+ (3) 4096 bytes from a buffer of size 8192. The last read is unaligned because your slice is 2048 bytes larger than what's left in the 8192 byte bucket. You propose that these last 2048 bytes are collected with an extra read call to the underlying reader.

If this is correct, it seems like a bad trade off to gain the convenience of a full slice at the expense of latency. The underlying reader could be expensive to read from.

If you want to fill the slice, why not use something like io.ReadAtLeast ?

Have you run benchmarks on your change to see how it affects performance in a non-trivial example?

[as]

In addition, consider the example when the length of p is 8192+1, should it really try to read that one last byte from the underlying data source just to return a full slice?

[paulstuart]

d'oh! ReadAtLeast addresses the issue and I completely overlooked that. Sorry for the trouble.