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

2022-05-24 Thread XenoAmess
On Fri, 20 May 2022 21:15:23 GMT, Roger Riggs  wrote:

>> 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 78:
> 
>> 76: SoftReference 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;

@RogerRiggs done.

-

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


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

2022-05-24 Thread XenoAmess
On Fri, 20 May 2022 21:08:51 GMT, Roger Riggs  wrote:

>> 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.

The "notice..." statement removed.

-

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


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

2022-05-24 Thread XenoAmess
On Fri, 20 May 2022 21:05:07 GMT, Roger Riggs  wrote:

>> 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 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)`.

done.

-

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


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

2022-05-20 Thread Roger Riggs
On Wed, 20 Apr 2022 16:52:31 GMT, XenoAmess  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 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


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

2022-04-20 Thread XenoAmess
> @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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5872/files
  - new: https://git.openjdk.java.net/jdk/pull/5872/files/47d8e195..fbc24743

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5872&range=11
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5872&range=10-11

  Stats: 8 lines in 1 file changed: 7 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5872.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5872/head:pull/5872

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