Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v7]
On Fri, 14 May 2021 15:57:48 GMT, Brian Burkhalter wrote:
> Any more comments on this? Thanks.
I think you've addressed the obvious bugs now but I'm still nervous that the
lseek will fail for some special devices (time will tell).
It would be good to do some cleanup on the test before you integrate this. If
you change it to use try-with-resources will it will avoid leaving the file
open when the test fails. Also would be good to add a test for readNBytes(0)
for the empty-file case. There's a typo in one of the exception messages
("expecte").
-
PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v7]
On Mon, 10 May 2021 17:54:52 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: Handle readNBytes(0): src/test New changes look good to me. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v7]
On Mon, 10 May 2021 17:54:52 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: Handle readNBytes(0): src/test Any more comments on this? Thanks. - PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v7]
> 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: Handle readNBytes(0): src/test - Changes: - all: https://git.openjdk.java.net/jdk/pull/3845/files - new: https://git.openjdk.java.net/jdk/pull/3845/files/5272d5ef..4fae0209 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3845&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3845&range=05-06 Stats: 6 lines in 2 files changed: 5 ins; 0 del; 1 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
