RFR: 8284638: store skip buffers in InputStream Object [v12]

Roger Riggs rriggs at openjdk.java.net
Fri May 20 21:18:53 UTC 2022


On Wed, 20 Apr 2022 16:52:31 GMT, XenoAmess <duke at openjdk.java.net> wrote:

>> @jmehrens what about this then?
>> I think it safe now(actually this mechanism is learned from Reader)
>
> XenoAmess has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add documents

src/java.base/share/classes/java/io/InputStream.java line 72:

> 70:      *
> 71:      * @param size minimum length that the skip byte array must have.
> 72:      *             notice that this param input MUST be equal or less than {@link #MAX_SKIP_BUFFER_SIZE MAX_SKIP_BUFFER_SIZE}.

The "MUST be equal" reads like something will go wrong if that condition isn't satisfied.
This method does not care about the size, except in potentially resizing if the requested size is larger.

The "notice..." statement is making a statement about code in the caller, and so doesn't belong here.

src/java.base/share/classes/java/io/InputStream.java line 75:

> 73:      * @return the byte array.
> 74:      */
> 75:     private byte[] skipBufferReference(int size) {

The method name would match the return value better if named `skipBuffer(size)`.

src/java.base/share/classes/java/io/InputStream.java line 78:

> 76:         SoftReference<byte[]> ref = this.skipBufferReference;
> 77:         byte[] buffer;
> 78:         if (ref == null || (buffer = ref.get()) == null || buffer.length < size) {

A comment here or in the method javadoc to the effect that a new buffer is allocated unless the existing buffer is large enough might head off doubt that buffer is always non-null at the return.  As would inverting the if:

   if (ref != null && (buffer = ref.get()) != null && buffer.length >= size) {
      return buffer;
   }
   // allocate new or larger buffer
   buffer = new byte[size];
   this.skipBufferReference = new SoftReference<>(buffer);
   return buffer;

-------------

PR: https://git.openjdk.java.net/jdk/pull/5872


More information about the core-libs-dev mailing list