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

[Paul Sandoz]

> On 19 Dec 2017, at 13:43, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:
> 
> On Dec 19, 2017, at 12:52 PM, Paul Sandoz <paul.sandoz at oracle.com <mailto:paul.sandoz at oracle.com>> wrote:
> 
>> For the case of reading 2^N bytes i believe you can avoid doing a last copy by checking if “n < 0" within the “nread > 0” block when “nread == DEAFULT_BUFFER_SIZE”. That might close the perf gap for smaller cases. You can also move "nread = 0” to the same block e.g.:
>> 
>>  var copy = (n < 0 && nread == DEAFULT_BUFFER_SIZE) ? buf : Arrays.copyOf(buf, nread);
>>  list.add(copy)
>>  nread = 0;
> 
> Definitely improves performance and memory footprint for this case.
> 
>> 262         byte[] output = new byte[total];
>> 263         int offset = 0;
>> 264         int numCached = list.size();
>> 265         for (int i = 0; i < numCached; i++) {
>> 266             byte[] b = list.get(i);
>> 267             System.arraycopy(b, 0, output, offset, b.length);
>> 268             offset += b.length;
>> 269         }
>> 
>> You can simplify to:
>> 
>> var result = new byte[total];
>> int offset = 0;
>> for (buf : list) {
>>  System.arraycopy(buf, 0, result, offset, buf.length);
>>  offset += buf.length;
>> }
> 
> Oh, yes, of course.
> 
>> s/list/bufs and then you can use var for the declarations at the start of the method.
> 
> Done. Updated: http://cr.openjdk.java.net/~bpb/8193832/webrev.01/ <http://cr.openjdk.java.net/~bpb/8193832/webrev.01/>
> 

You can also simplify the “for(;;) + break" into a do while loop:

do {
  int nread = 0;
  ...
} while (n > 0);


> Good suggestions! Not sure however about line 237 as with var it has to be “var n = 0;” instead of simply “int n;” with no initialization.

I was only suggesting it’s use for the byte[] and ArrayList<byte[]>. IMHO it’s a little subjective but there is less upside for int, although one can argue consistent application and explicit initialization is a good thing here.


> Also I’ve not tested the effect of the initial List capacity at line 233: the current value of 128 is arbitrary - might be better to go with the default?
> 

My inclination would be to use 8 instead of 128, that allows 2^16 in size by default, rather than 2^20.

Paul.