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

Peter Levart peter.levart at gmail.com
Thu Dec 21 10:03:53 UTC 2017


Hi Brian,

On 12/20/2017 11:30 PM, Brian Burkhalter wrote:
> On Dec 20, 2017, at 11:52 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>
>> 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.
>
> http://cr.openjdk.java.net/~bpb/8193832/webrev.03/
>
> The patch is updated to:
>
> * use Peter’s approach to avoid allocating an ArrayList when length <= DEFAULT_BUFFER_SIZE;
> * use the default ArrayList constructor instead of that with a specific initial capacity;
> * update the test to ensure that lengths which require three buffers are covered.
>
> Thanks,
>
> Brian

This is OK as is, but I see another possible improvement to the logic. 
You decide whether it is worth trying to implement it. Currently the 
logic reads stream into buffers of DEFAULT_BUFFER_SIZE and adds them to 
an ArrayList, except the last buffer which is 1st copied into a shorter 
buffer before being appended to the list. This copying is unnecessary. 
The copied buffer has the same content, but shorter length. But the 
information about the length of final buffer is contained elsewhere too 
(for example implicitly in 'total'). So you copuld change the final 
"gathering" loop to extract this information for the final buffer and 
there would be no redundant copying of final buffer necessary.

What do you think?

Regards, Peter




More information about the core-libs-dev mailing list