Re: RFR: 8284638: store skip buffers in InputStream Object [v12]
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]
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]
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]
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]
> @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
