io/ioutil: don't cap buffer size in ReadFile

When we added a Stat call to determine the initial buffer size in
https://golang.org/cl/163069, we included an arbitrary 1e9-byte limit
"just in case". That interacts badly with power-of-2 resizing in
*bytes.Buffer: it causes buffers reading from very large files to
consume up to twice the necessary space.

The documentation for (os.FileInfo).Size says that it reports "length
in bytes for regular files; system-dependent for others", but the
"system dependent" cases overwhelmingly return either a small number
(e.g., the length of the target path for a symlink) or a non-positive
number (e.g., for a file in /proc under Linux). It should be
appropriate to use the number reported by Size as an approximate lower
bound, even if it is large.

fixes #21455

Change-Id: I609c72519b7b87428c24d0b22db46eede30e0e54
Reviewed-on: https://go-review.googlesource.com/55870
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/io/ioutil/ioutil.go b/src/io/ioutil/ioutil.go
index f0da6168305d60a59d4f909ba1749109a7d97194..674b2701db1c15873a03e4f1c8796c88aac9f439 100644 (file)
--- a/src/io/ioutil/ioutil.go
+++ b/src/io/ioutil/ioutil.go
@@ -16,7 +16,7 @@ import (
 // readAll reads from r until an error or EOF and returns the data it read
 // from the internal buffer allocated with a specified capacity.
 func readAll(r io.Reader, capacity int64) (b []byte, err error) {
-       buf := bytes.NewBuffer(make([]byte, 0, capacity))
+       var buf bytes.Buffer
        // If the buffer overflows, we will get bytes.ErrTooLarge.
        // Return that as an error. Any other panic remains.
        defer func() {
@@ -30,6 +30,9 @@ func readAll(r io.Reader, capacity int64) (b []byte, err error) {
                        panic(e)
                }
        }()
+       if int64(int(capacity)) == capacity {
+               buf.Grow(int(capacity))
+       }
        _, err = buf.ReadFrom(r)
        return buf.Bytes(), err
 }
@@ -54,20 +57,20 @@ func ReadFile(filename string) ([]byte, error) {
        defer f.Close()
        // It's a good but not certain bet that FileInfo will tell us exactly how much to
        // read, so let's try it but be prepared for the answer to be wrong.
-       var n int64
+       var n int64 = bytes.MinRead
 
        if fi, err := f.Stat(); err == nil {
-               // Don't preallocate a huge buffer, just in case.
-               if size := fi.Size(); size < 1e9 {
+               // As initial capacity for readAll, use Size + a little extra in case Size
+               // is zero, and to avoid another allocation after Read has filled the
+               // buffer. The readAll call will read into its allocated internal buffer
+               // cheaply. If the size was wrong, we'll either waste some space off the end
+               // or reallocate as needed, but in the overwhelmingly common case we'll get
+               // it just right.
+               if size := fi.Size() + bytes.MinRead; size > n {
                        n = size
                }
        }
-       // As initial capacity for readAll, use n + a little extra in case Size is zero,
-       // and to avoid another allocation after Read has filled the buffer. The readAll
-       // call will read into its allocated internal buffer cheaply. If the size was
-       // wrong, we'll either waste some space off the end or reallocate as needed, but
-       // in the overwhelmingly common case we'll get it just right.
-       return readAll(f, n+bytes.MinRead)
+       return readAll(f, n)
 }
 
 // WriteFile writes data to a file named by filename.