From 1d07ed15798b1228a2777c8ad80353b17a9ad8bd Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 15 Aug 2017 15:08:39 -0400 Subject: [PATCH] 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 Reviewed-by: Joe Tsai Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot --- src/io/ioutil/ioutil.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/io/ioutil/ioutil.go b/src/io/ioutil/ioutil.go index f0da616830..674b2701db 100644 --- 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. -- 2.50.0