RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

Limit native memory allocation and move write loop from the native layer into Java. This change should make the OOME reported in the issue much less likely. - Commit messages: - 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available Changes: https:/

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

On Fri, 21 Jul 2023 22:40:00 GMT, Brian Burkhalter wrote: > Limit native memory allocation and move write loop from the native layer into > Java. This change should make the OOME reported in the issue much less likely. The cost of native memory allocation appears to degrade the throughput of re

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

On Fri, 21 Jul 2023 22:40:00 GMT, Brian Burkhalter wrote: > Limit native memory allocation and move write loop from the native layer into > Java. This change should make the OOME reported in the issue much less likely. src/java.base/share/native/libjava/io_util.c line 99: > 97: return

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

On Fri, 21 Jul 2023 22:40:00 GMT, Brian Burkhalter wrote: > Limit native memory allocation and move write loop from the native layer into > Java. This change should make the OOME reported in the issue much less likely. src/java.base/share/classes/java/io/FileOutputStream.java line 366: > 364:

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

On Mon, 24 Jul 2023 09:16:23 GMT, Vyom Tewari wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > src/java.base/share/native/libjava/io_util.c line 99: > >> 97:

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

: Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available On Mon, 24 Jul 2023 09:16:23 GMT, Vyom Tewari wrote: >> Limit native memory allocation and move write loop from the native layer into Java. This change should make the OOME reported in the issu

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

I think the idea is to treat an IOException thrown by any but the first invocation of readBytes() as equivalent to end-of-file such as described for InputStream::read(byte[],int,int) w

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

On Mon, 24 Jul 2023 12:59:55 GMT, Alan Bateman wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > src/java.base/share/classes/java/io/FileOutputStream.java line 366: >

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

On Mon, 24 Jul 2023 13:16:56 GMT, Alan Bateman wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > src/java.base/share/native/libjava/io_util.c line 62: > >> 60: /* The

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available

On Tue, 25 Jul 2023 02:09:29 GMT, Brian Burkhalter wrote: > It's based on micro-benchmarks. Having the loops in Java reduces throughput > but allocating memory using `malloc(len)` also reduces throughput as `len` > gets larger and this threshold appears to balance the two. Are these micro benc

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v6]

> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. Brian Burkhalter has updated the pull request with a new target base due to a merge or a rebase.

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v5]

On Thu, 9 Jun 2022 18:38:18 GMT, Brian Burkhalter wrote: >> Modify native multi-byte read-write code used by the `java.io` classes to >> limit the size of the allocated native buffer thereby decreasing off-heap >> memory footprint and increasing throughput. > > Brian Burkhalter has updated the

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v7]

> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. Brian Burkhalter has updated the pull request with a new target base due to a merge or a rebase.

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v7]

On Wed, 20 Jul 2022 23:41:04 GMT, Brian Burkhalter wrote: >> Modify native multi-byte read-write code used by the `java.io` classes to >> limit the size of the allocated native buffer thereby decreasing off-heap >> memory footprint and increasing throughput. > > Brian Burkhalter has updated the

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v7]

On Wed, 3 Aug 2022 12:54:38 GMT, Daniel Fuchs wrote: >> Brian Burkhalter has updated the pull request with a new target base due to >> a merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 12 additional >> commit

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v7]

On Wed, 3 Aug 2022 15:42:12 GMT, Brian Burkhalter wrote: >> src/java.base/share/classes/java/io/RandomAccessFile.java line 112: >> >>> 110: } >>> 111: >>> 112: int multiple = (len + MIN_BUFFER_SIZE - 1)/MIN_BUFFER_SIZE; >> >> isn't that equivalent `(len / MIN_BUFFER_SIZE) + 1`,

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v8]

> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. Brian Burkhalter has updated the pull request with a new target base due to a merge or a rebase.

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v9]

> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. Brian Burkhalter has updated the pull request with a new target base due to a merge or a rebase.

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v8]

On Wed, 3 Aug 2022 17:14:13 GMT, Brian Burkhalter wrote: >> Modify native multi-byte read-write code used by the `java.io` classes to >> limit the size of the allocated native buffer thereby decreasing off-heap >> memory footprint and increasing throughput. > > Brian Burkhalter has updated the

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v9]

On Fri, 26 Aug 2022 22:20:24 GMT, Brian Burkhalter wrote: >> Modify native multi-byte read-write code used by the `java.io` classes to >> limit the size of the allocated native buffer thereby decreasing off-heap >> memory footprint and increasing throughput. > > Brian Burkhalter has updated the

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Tue, 25 Jul 2023 02:05:52 GMT, Brian Burkhalter wrote: >> src/java.base/share/classes/java/io/FileOutputStream.java line 366: >> >>> 364: int n = writeBytes(b, off, len, append); >>> 365: if (n == -1) >>> 366: break; >> >> Checking if n is

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

> Limit native memory allocation and move write loop from the native layer into > Java. This change should make the OOME reported in the issue much less likely. Brian Burkhalter has updated the pull request incrementally with three additional commits since the last revision: - 6478546: Decreas

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Mon, 24 Jul 2023 13:04:56 GMT, Alan Bateman wrote: >> src/java.base/share/native/libjava/io_util.c line 99: >> >>> 97: return 0; >>> 98: } else if (len > BUF_SIZE) { >>> 99: if (len > MAX_MALLOC_SIZE) >> >> Hi Brian if I am reading code correctly then with the current cod

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Tue, 25 Jul 2023 05:51:26 GMT, Alan Bateman wrote: >> It's based on micro-benchmarks. Having the loops in Java reduces throughput >> but allocating memory using `malloc(len)` also reduces throughput as `len` >> gets larger and this threshold appears to balance the two. > >> It's based on mic

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Tue, 25 Jul 2023 23:50:07 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with th

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Tue, 25 Jul 2023 23:50:07 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with th

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Tue, 25 Jul 2023 23:50:07 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with th

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Wed, 26 Jul 2023 06:17:13 GMT, Vyom Tewari wrote: > If i am reading code correctly then with the new implementation, until client > issue the next "FIS.read" he may or may not know if there was exception > pending in previous 'read' call ?. > I am not sure in case of partial read we have sup

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Wed, 26 Jul 2023 00:00:33 GMT, Brian Burkhalter wrote: > There was no dropping of the file system cache. I would think this would have > more of an effect on measuring the throughput of writing than reading. In that case, I assume the benchmarks are just reading from the file system cache s

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Tue, 25 Jul 2023 23:50:07 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with th

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Wed, 26 Jul 2023 05:26:52 GMT, Vyom Tewari wrote: >> Brian Burkhalter has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - 6478546: Decrease malloc limit to 1.5 MB >> - 6478546: Minor refactoring >> - 6478546: Prevent short read > >

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Wed, 26 Jul 2023 09:29:18 GMT, Alan Bateman wrote: > I think the main issue with this version is that it changes behavior of the > read methods to work like "read fully", I don't think we should do that. To me it looks more like `readNBytes`. - PR Comment: https://git.openjdk.o

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Wed, 26 Jul 2023 05:31:57 GMT, Vyom Tewari wrote: > please reformat line 173 Why? - PR Review Comment: https://git.openjdk.org/jdk/pull/14981#discussion_r1275174632

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Wed, 26 Jul 2023 15:49:38 GMT, Brian Burkhalter wrote: >> src/java.base/share/native/libjava/io_util.c line 173: >> >>> 171: if (len > MAX_MALLOC_SIZE) >>> 172: len = MAX_MALLOC_SIZE; >>> 173: buf = (char*)malloc(len*sizeof(char)); >> >> please reformat line 173 >

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Wed, 26 Jul 2023 15:58:15 GMT, Vyom Tewari wrote: >>> please reformat line 173 >> >> Why? > > "buf = (char*)malloc(len*sizeof(char));" --> "buf = (char*)malloc(len * > sizeof(char));" > "buf = (char*)malloc(len_sizeof(char));" --> "buf = (char_)malloc(len * > sizeof(char));" Both sides of

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Tue, 25 Jul 2023 23:50:07 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with th

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Tue, 25 Jul 2023 23:50:07 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with th

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Tue, 25 Jul 2023 23:50:07 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with th

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Thu, 27 Jul 2023 13:42:43 GMT, Sergey Tsypanov wrote: >> Brian Burkhalter has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - 6478546: Decrease malloc limit to 1.5 MB >> - 6478546: Minor refactoring >> - 6478546: Prevent short read

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Thu, 27 Jul 2023 15:54:30 GMT, Brian Burkhalter wrote: >> src/java.base/share/native/libjava/io_util.c line 199: >> >>> 197: } >>> 198: >>> 199: if (buf != stackBuf) >> >> Wouldn't this cause a leak when if-condition is not met and `free(buf)` is >> not called? > > I don't see how

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Thu, 27 Jul 2023 16:35:05 GMT, Sergey Tsypanov wrote: >> I don't see how this is possible. The value of `buf` is either `stackBuf` or >> a value returned by `malloc()`. In any case, this code will be superseded. > > Then I guess we don't need this `if`-clause Then I think one gets an error i

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Thu, 27 Jul 2023 16:59:58 GMT, Brian Burkhalter wrote: >> Then I guess we don't need this `if`-clause > > Then I think one gets an error if `0 < len < BUF_SIZE`: > > > $ cat free.c > #include > > int main(int argc, char** argv) > { > char stackBuf[8]; > char* buf; > > buf = st

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v3]

> Limit native memory allocation and move write loop from the native layer into > Java. This change should make the OOME reported in the issue much less likely. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 6478546: Move buffer

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

On Wed, 26 Jul 2023 09:29:18 GMT, Alan Bateman wrote: > I think this is close to what you want Modified as suggested in 9264975. The limit is increased to 1.5 Mb where it was in native malloc clamping. This appears to prevent any throughput degradation. - PR Comment: https://git.o

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v3]

On Fri, 28 Jul 2023 19:59:15 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with on

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v3]

On Sun, 30 Jul 2023 04:54:32 GMT, Vyom Tewari wrote: >> Brian Burkhalter has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 6478546: Move buffer clamping up to Java layer; correct read behavior to >> match legacy > > src/java.base/share/na

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v3]

On Fri, 28 Jul 2023 19:59:15 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with on

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v4]

> Limit native memory allocation and move write loop from the native layer into > Java. This change should make the OOME reported in the issue much less likely. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 6478546: do-while ->

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v3]

On Sun, 30 Jul 2023 13:06:36 GMT, Alan Bateman wrote: >> Brian Burkhalter has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 6478546: Move buffer clamping up to Java layer; correct read behavior to >> match legacy > > src/java.base/share/c

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v4]

On Mon, 31 Jul 2023 17:02:15 GMT, Brian Burkhalter wrote: >> Limit native memory allocation and move write loop from the native layer >> into Java. This change should make the OOME reported in the issue much less >> likely. > > Brian Burkhalter has updated the pull request incrementally with on

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v5]

> Limit native memory allocation and move write loop from the native layer into > Java. This change should make the OOME reported in the issue much less likely. Brian Burkhalter has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the un

Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v6]

> Limit native memory allocation and move write loop from the native layer into > Java. This change should make the OOME reported in the issue much less likely. Brian Burkhalter has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the un