RFR: 8147468: Allow users to bound the size of buffers cached in the per-thread buffer caches

[Tony Printezis]

Alan,

Thanks for the comments. Please take a look at:

http://cr.openjdk.java.net/~tonyp/nio-max-buffer-size/webrev.3/

* changed jdk.nio.MaxCachedBufferSize to jdk.nio.maxCachedBufferSize
* changed the copyright to the right (I hope!) version and put the right copyright dates
* test now checks hasRemaining() instead of catching BufferOverflowException (I also had to change the buffer put to be done as bytes, instead of ints, given that ints are 4-byte aligned and the calculated buffer size was not)
* I now create a FileChannel directly

Tony

On January 19, 2016 at 10:09:37 AM, Alan Bateman (alan.bateman at oracle.com) wrote:



On 15/01/2016 16:54, Tony Printezis wrote:
(I’m starting a new thread so that it’s obvious this is a code review request) Second version of this change, addressing Alan’s comments (see previous e-mail exchange):

http://cr.openjdk.java.net/~tonyp/nio-max-buffer-size/webrev.2/


I think this mostly looks okay. I assume the property should be jdk.nio.maxCachedBufferSize so that the casing is consistent with other properties. As this will be documented/supported property then I'll need to get a CCC submitted.

A few comments on the test:

1. The copyright headers needs to be the pure GPL header, we don't use the Classpath exception on tests. Also the date is duplicated.

2. Using BufferOverflowException when filling the buffer with random bytes is a bit icky, it could use while buffer.hasRemaining() instead. Alternatively random.nextBytes(buffer.array()) should do it in one-line.

3. If you want then you could create the FileChannel directly, the RandomAccessFile + getChannel is not needed.

I think that's it.

-Alan.



-----

Tony Printezis | JVM/GC Engineer / VM Team | Twitter

@TonyPrintezis
tprintezis at twitter.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20160121/a51b18c8/attachment.html>