RFR 8193832: Performance of InputStream.readAllBytes() could be improved (original) (raw)

Paul Sandoz paul.sandoz at oracle.com
Wed Dec 20 19:52:44 UTC 2017

On 20 Dec 2017, at 11:04, John Rose <john.r.rose at oracle.com> wrote:

On Dec 20, 2017, at 3:45 AM, Peter Levart <peter.levart at gmail.com <mailto:peter.levart at gmail.com>> wrote:

allocation of ArrayList could be avoided. Imagine a use case where lots of small files are read into byte[] arrays +1 I was going to write a similar suggestion; thanks for sending it out.

I was a little lassiaz-faire given that 8K bytes were anyway being allocated upfront. Peter’s changes look good.

Brian, i would double check the tests to make sure the various code paths are tested.

Paul.

These sorts of things often need to be designed to perform well at all scales, not just large scales.

The new ArrayList(8) is dead weight if the data fits in the buffer. So it's bad for scale < buffer length._ _The earlier new ArrayList(128) is a noticeable overhead if the data_ _fits in two buffers. So it's not so good for scale = M * (buffer length)_ _where M is about 2._ _For a large scale result (M > 10), the footprint difference between ArrayList(N) for various N is a second-order fraction. — John P.S. Road not taken: Instead of new ArrayList(8) you could use the default ArrayList constructor, and allocate it unconditionally. It has a very low overhead if the ArrayList remains empty. Using that constructor could motivate you to simplify the code to use the ArrayList unconditionally, since that would be just a single heap node if it is not used to gather multiple results. But I like the "null" formulation despite its complexity. So I'd probably keep it the way Peter wrote it.

More information about the core-libs-dev mailing list