> On 19 Dec 2017, at 13:43, Brian Burkhalter <brian.burkhal...@oracle.com>
> wrote:
>
> On Dec 19, 2017, at 12:52 PM, Paul Sandoz <paul.san...@oracle.com
> <mailto:paul.san...@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.
