Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v2]
On Thu, 1 Jul 2021 21:59:33 GMT, Markus KARG wrote: >> This PR-*draft* is **work in progress** and an invitation to discuss a >> possible solution for issue >> [JDK-8265891](https://bugs.openjdk.java.net/browse/JDK-8265891). It is *not >> yet* intended for a final review. >> >> As proposed in JDK-8265891, this PR provides an implementation for >> `Channels.newInputStream().transferTo()` which provide superior performance >> compared to the current implementation. The changes are: >> * Prevents transfers through the JVM heap as much as possibly by offloading >> to deeper levels via NIO, hence allowing the operating system to optimize >> the transfer. >> * Using more JRE heap in the fallback case when no NIO is possible (still >> only KiBs, hence mostl ynegligible even on SBCs) to better perform on modern >> hardware / fast I/O devides. >> >> Using JMH I have benchmarked both, the original implementation and this >> implementation, and (depending on the used hardware and use case) >> performance change was approx. doubled performance. So this PoC proofs that >> it makes sense to finalize this work and turn it into an actual OpenJDK >> contribution. >> >> I encourage everybody to discuss this draft: >> * Are there valid arguments for *not* doing this change? >> * Is there a *better* way to improve performance of >> `Channels.newInputStream().transferTo()`? >> * How to go on from here: What is missing to get this ready for an actual >> review? > > Markus KARG has updated the pull request incrementally with one additional > commit since the last revision: > > Draft: Renaming i and separating code into several methods > > Signed-off-by: Markus Karg I have renamed `i` (and other ambiguous variables) and separated the implementation of `transferTo` into multiple methods. I will look into the test examples next to learn what I can reuse when writing tests for all those implementations. Thank you for all the kind input and the good ideas. - PR: https://git.openjdk.java.net/jdk/pull/4263
Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v2]
> This PR-*draft* is **work in progress** and an invitation to discuss a > possible solution for issue > [JDK-8265891](https://bugs.openjdk.java.net/browse/JDK-8265891). It is *not > yet* intended for a final review. > > As proposed in JDK-8265891, this PR provides an implementation for > `Channels.newInputStream().transferTo()` which provide superior performance > compared to the current implementation. The changes are: > * Prevents transfers through the JVM heap as much as possibly by offloading > to deeper levels via NIO, hence allowing the operating system to optimize the > transfer. > * Using more JRE heap in the fallback case when no NIO is possible (still > only KiBs, hence mostl ynegligible even on SBCs) to better perform on modern > hardware / fast I/O devides. > > Using JMH I have benchmarked both, the original implementation and this > implementation, and (depending on the used hardware and use case) performance > change was approx. doubled performance. So this PoC proofs that it makes > sense to finalize this work and turn it into an actual OpenJDK contribution. > > I encourage everybody to discuss this draft: > * Are there valid arguments for *not* doing this change? > * Is there a *better* way to improve performance of > `Channels.newInputStream().transferTo()`? > * How to go on from here: What is missing to get this ready for an actual > review? Markus KARG has updated the pull request incrementally with one additional commit since the last revision: Draft: Renaming i and separating code into several methods Signed-off-by: Markus Karg - Changes: - all: https://git.openjdk.java.net/jdk/pull/4263/files - new: https://git.openjdk.java.net/jdk/pull/4263/files/ed49098a..7d18ca62 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4263=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4263=00-01 Stats: 108 lines in 1 file changed: 57 ins; 39 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/4263.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4263/head:pull/4263 PR: https://git.openjdk.java.net/jdk/pull/4263
