Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
On Mon, 15 Mar 2021 11:41:07 GMT, Alan Bateman wrote: >> Done > >> Done > > I think I'd prefer if the exception message would say that the JMOD is > invalid or that the JMOD file is truncated (because I don't think anyone > seeing this exception will know anything about the header). Fixed - PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
> In some cases wrapping of `InputStream` with `BufferedInputStream` is > redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does > not require any buffer having one within. > > Other cases are related to reading either a byte or short `byte[]`: in both > cases `BufferedInputStream.fill()` will be called resulting in load of much > bigger amount of data (8192 by default) than required. Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision: Fix error message - Changes: - all: https://git.openjdk.java.net/jdk/pull/2992/files - new: https://git.openjdk.java.net/jdk/pull/2992/files/f69c8ff4..ff25cc4d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2992&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2992&range=03-04 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2992.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2992/head:pull/2992 PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
On Mon, 15 Mar 2021 12:19:19 GMT, Сергей Цыпанов wrote: >> In some cases wrapping of `InputStream` with `BufferedInputStream` is >> redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which >> does not require any buffer having one within. >> >> Other cases are related to reading either a byte or short `byte[]`: in both >> cases `BufferedInputStream.fill()` will be called resulting in load of much >> bigger amount of data (8192 by default) than required. > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > Fix error message Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
On Mon, 15 Mar 2021 12:19:19 GMT, Сергей Цыпанов
wrote:
>> In some cases wrapping of `InputStream` with `BufferedInputStream` is
>> redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which
>> does not require any buffer having one within.
>>
>> Other cases are related to reading either a byte or short `byte[]`: in both
>> cases `BufferedInputStream.fill()` will be called resulting in load of much
>> bigger amount of data (8192 by default) than required.
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix error message
Changes requested by dfuchs (Reviewer).
src/java.base/share/classes/sun/net/www/http/HttpClient.java line 434:
> 432: int r = tmpbuf.read();
> 433: if (r == -1) {
> 434: logFinest("HttpClient.available(): " +
There are some subtle things going on there. Using a `BufferedInputStream`
ensures that all the bytes available on the socket will be read, up to the
buffer capacity. Can you revert this change? I'd rather that this clean up be
handled separately. I have logged
https://bugs.openjdk.java.net/browse/JDK-8263599.
-
PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
On Mon, 15 Mar 2021 15:04:49 GMT, Daniel Fuchs wrote:
>> Сергей Цыпанов has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix error message
>
> src/java.base/share/classes/sun/net/www/http/HttpClient.java line 434:
>
>> 432: int r = tmpbuf.read();
>> 433: if (r == -1) {
>> 434: logFinest("HttpClient.available(): " +
>
> There are some subtle things going on there. Using a `BufferedInputStream`
> ensures that all the bytes available on the socket will be read, up to the
> buffer capacity. Can you revert this change? I'd rather that this clean up be
> handled separately. I have logged
> https://bugs.openjdk.java.net/browse/JDK-8263599.
Sure, I'll revert this.
-
PR: https://git.openjdk.java.net/jdk/pull/2992
