Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v6]

2021-03-15 Thread Sergey Bylokhov
On Mon, 15 Mar 2021 18:04:26 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

Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy

2021-03-15 Thread Julia Boes
On Thu, 18 Feb 2021 07:12:34 GMT, Andrey Turbanov wrote: >> Hi @turbanoff, I'm happy to sponsor but I see two comments by @marschall - >> have they been addressed? >> https://github.com/openjdk/jdk/pull/1853#discussion_r572815422 >>

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v6]

2021-03-15 Thread Daniel Fuchs
On Mon, 15 Mar 2021 18:01:20 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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v6]

2021-03-15 Thread Сергей Цыпанов
> 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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]

2021-03-15 Thread Сергей Цыпанов
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:

Re: RFR: 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll

2021-03-15 Thread Chris Hegarty
On Sun, 14 Mar 2021 12:29:45 GMT, Alan Bateman wrote: >> This is an adaptation of a patch originally written by Shafi Ahmad in >> a comment on the JBS page but never submitted or merged. >> >> With -Xcheck:jni, the method java.net.NetworkInterface.getAll very >> quickly breaches the default JNI

Re: RFR: 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll [v3]

2021-03-15 Thread Chris Hegarty
On Mon, 15 Mar 2021 16:59:24 GMT, Jonathan Dowland wrote: >> This is an adaptation of a patch originally written by Shafi Ahmad in >> a comment on the JBS page but never submitted or merged. >> >> With -Xcheck:jni, the method java.net.NetworkInterface.getAll very >> quickly breaches the default

Re: RFR: 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll [v3]

2021-03-15 Thread Jonathan Dowland
> This is an adaptation of a patch originally written by Shafi Ahmad in > a comment on the JBS page but never submitted or merged. > > With -Xcheck:jni, the method java.net.NetworkInterface.getAll very > quickly breaches the default JNI local refs threshold (32). Exactly when > this happens

Re: RFR: 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll [v2]

2021-03-15 Thread Jonathan Dowland
On Mon, 15 Mar 2021 12:53:11 GMT, Chris Hegarty wrote: >> Jonathan Dowland has refreshed the contents of this pull request, and >> previous commits have been removed. The incremental views will show >> differences compared to the previous content of the PR. > >

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]

2021-03-15 Thread Daniel Fuchs
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

Re: RFR: 8263442: Potential bug in jdk.internal.net.http.common.Utils.CONTEXT_RESTRICTED

2021-03-15 Thread Michael McMahon
On Mon, 15 Mar 2021 13:21:25 GMT, Daniel Fuchs wrote: >> Hi, >> >> The fix for the reported bug in Utils.CONTEXT_RESTRICTED caused a couple of >> regression failures, which turned out to be another bug exposed by this fix >> where HTTP/1.1 CONNECT requests with authentication were filtering

Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v13]

2021-03-15 Thread Andrey Turbanov
> 8080272 Refactor I/O stream copying to use > InputStream.transferTo/readAllBytes and Files.copy Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]

2021-03-15 Thread Alan Bateman
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

Re: RFR: 8263442: Potential bug in jdk.internal.net.http.common.Utils.CONTEXT_RESTRICTED

2021-03-15 Thread Daniel Fuchs
On Fri, 12 Mar 2021 21:04:59 GMT, Michael McMahon wrote: > Hi, > > The fix for the reported bug in Utils.CONTEXT_RESTRICTED caused a couple of > regression failures, which turned out to be another bug exposed by this fix > where HTTP/1.1 CONNECT requests with authentication were filtering out

Re: RFR: 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll [v2]

2021-03-15 Thread Jonathan Dowland
> This is an adaptation of a patch originally written by Shafi Ahmad in > a comment on the JBS page but never submitted or merged. > > With -Xcheck:jni, the method java.net.NetworkInterface.getAll very > quickly breaches the default JNI local refs threshold (32). Exactly when > this happens

Re: RFR: 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll [v2]

2021-03-15 Thread Chris Hegarty
On Mon, 15 Mar 2021 12:53:16 GMT, Jonathan Dowland wrote: >> This is an adaptation of a patch originally written by Shafi Ahmad in >> a comment on the JBS page but never submitted or merged. >> >> With -Xcheck:jni, the method java.net.NetworkInterface.getAll very >> quickly breaches the default

Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy

2021-03-15 Thread Julia Boes
On Thu, 18 Feb 2021 07:12:34 GMT, Andrey Turbanov wrote: >> Hi @turbanoff, I'm happy to sponsor but I see two comments by @marschall - >> have they been addressed? >> https://github.com/openjdk/jdk/pull/1853#discussion_r572815422 >>

Re: RFR: 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll

2021-03-15 Thread Jonathan Dowland
Hi, Thanks for your review! On Sun, Mar 14, 2021 at 12:32:07PM +, Alan Bateman wrote: > I think this looks okay. I assume the deletion of the locals refs at > the end of createNetworkInterface aren't really needed, but harmless. For the case where the JNI entrypoint is

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]

2021-03-15 Thread Сергей Цыпанов
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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]

2021-03-15 Thread Сергей Цыпанов
> 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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v4]

2021-03-15 Thread Alan Bateman
On Sun, 14 Mar 2021 19:32:11 GMT, Сергей Цыпанов wrote: > 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). - PR:

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v4]

2021-03-15 Thread Сергей Цыпанов
> 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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v3]

2021-03-15 Thread Сергей Цыпанов
On Sun, 14 Mar 2021 22:48:18 GMT, Sergey Bylokhov wrote: >> Сергей Цыпанов has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Use InputStream.readNBytes() and fix JLinkNegativeTest > >

Re: RFR: 8263561: Re-examine uses of LinkedList

2021-03-15 Thread Alan Bateman
On Sun, 14 Mar 2021 17:26:07 GMT, Alan Bateman wrote: >> Looks like it's never specified in JavaDoc which particular implementation >> of List is used in fields of affected classes, so it's quite odd to me that >> someone would rely on that when using reflection. But your point about >>