Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v6]
On Thu, 6 May 2021 16:40:31 GMT, Brian Burkhalter wrote: >> Please consider this request to override the `java.io.InputStream` methods >> `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more >> performant implementations. The method overrides attempt to read all >> requested bytes into a single array of the required size rather than >> composing the result from a sequence of smaller arrays. An example of the >> performance improvements is as follows. >> >> **readAllBytes** >> Before >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031 >> ± 7.309 ops/s >> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181 >> ± 0.369 ops/s >> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860 >> ± 0.048 ops/s >> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298 >> ± 0.183 ops/s >> >> After >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473 >> ± 43.425 ops/s >> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781 >> ± 1.273 ops/s >> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461 >> ± 0.084 ops/s >> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502 >> ± 0.003 ops/s >> >> >> **readNBytes** >> >> `N = file_size / 2` >> >> Before >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500 >> ± 153.353 ops/s >> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164 >> ± 1.104 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167 >> ± 0.141 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291 >> ± 0.211 ops/s >> >> >> After >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210 >> ± 66.045 ops/s >> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752 >> ± 0.951 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043 >> ± 0.102 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629 >> ± 0.035 ops/s > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8264777: Make length and position consistent with RAF; add path to OOME > message src/java.base/share/classes/java/io/FileInputStream.java line 319: > 317: } > 318: > 319: public byte[] readNBytes(int len) throws IOException { readNBytes(0) is specified to return an empty array, it looks like this implementation will throw IIOBE. Can you check this? - PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v6]
On Thu, 6 May 2021 16:40:31 GMT, Brian Burkhalter wrote: >> Please consider this request to override the `java.io.InputStream` methods >> `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more >> performant implementations. The method overrides attempt to read all >> requested bytes into a single array of the required size rather than >> composing the result from a sequence of smaller arrays. An example of the >> performance improvements is as follows. >> >> **readAllBytes** >> Before >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031 >> ± 7.309 ops/s >> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181 >> ± 0.369 ops/s >> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860 >> ± 0.048 ops/s >> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298 >> ± 0.183 ops/s >> >> After >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473 >> ± 43.425 ops/s >> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781 >> ± 1.273 ops/s >> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461 >> ± 0.084 ops/s >> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502 >> ± 0.003 ops/s >> >> >> **readNBytes** >> >> `N = file_size / 2` >> >> Before >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500 >> ± 153.353 ops/s >> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164 >> ± 1.104 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167 >> ± 0.141 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291 >> ± 0.211 ops/s >> >> >> After >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210 >> ± 66.045 ops/s >> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752 >> ± 0.951 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043 >> ± 0.102 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629 >> ± 0.035 ops/s > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8264777: Make length and position consistent with RAF; add path to OOME > message All comments have been addressed. Are there any more? Thanks. - PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v6]
> Please consider this request to override the `java.io.InputStream` methods > `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more > performant implementations. The method overrides attempt to read all > requested bytes into a single array of the required size rather than > composing the result from a sequence of smaller arrays. An example of the > performance improvements is as follows. > > **readAllBytes** > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031 > ± 7.309 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181 > ± 0.369 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860 > ± 0.048 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298 > ± 0.183 ops/s > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473 > ± 43.425 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781 > ± 1.273 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461 > ± 0.084 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502 > ± 0.003 ops/s > > > **readNBytes** > > `N = file_size / 2` > > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500 > ± 153.353 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164 > ± 1.104 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167 > ± 0.141 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291 > ± 0.211 ops/s > > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210 > ± 66.045 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752 > ± 0.951 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043 > ± 0.102 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629 > ± 0.035 ops/s Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8264777: Make length and position consistent with RAF; add path to OOME message - Changes: - all: https://git.openjdk.java.net/jdk/pull/3845/files - new: https://git.openjdk.java.net/jdk/pull/3845/files/0e476822..5272d5ef Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3845&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3845&range=04-05 Stats: 13 lines in 2 files changed: 7 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/3845.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3845/head:pull/3845 PR: https://git.openjdk.java.net/jdk/pull/3845