On Sun, 1 Aug 2021 22:01:33 GMT, Markus KARG <github.com+1701815+mk...@openjdk.org> 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 two additional > commits since the last revision: > > - Draft: Eliminated duplicate code using lambda expressions > - Draft: Use blocking mode also for target channel > I am a bit disappointed actually about that destructive answer at that late > point in time, now that I worked for months on all the requested changes and > tests. To prevent exactly this situation, I deliberately had the discussion > started in JIRA only, and I deliberately had the original code being just a > draft in the first place, and I deliberately did nearly _everything_ I was > asked to (including even the most irrelevant minor code style issues). And > you come up with the request to drop the code **now**? > > Certainly we could reduce the PR to just file channels, but in fact, now that > I spent all the time in the non-file-channels, I wonder why I shall throw > away all that work and go just with file channels actually? What is not > covered that was originally covered, and what is that lots of issues you talk > about? Actually I cannot see the actual problem unless you name it. There are 78 comments on this PR so far. We've tried to point out the bugs and issues at each iteration. We asked for tests because the changes introduce several code paths and implementations that would not be exercised by existing tests. There are several scenarios still missing and the patch doesn't yet have the microbenchmarks to demonstrate the improvements. I assume this is your first contribution so there will be learning curve and maybe some frustration. I think you have a better chance of success if you split this up and reduce the scope of this PR down to something manageable. Keeping the selectable channels out of this PR and focusing on the case where the input and output streams wrap file channels should make it simpler and may lead to a better solution. Reducing the scope will also reduce the burden on reviewers. ------------- PR: https://git.openjdk.java.net/jdk/pull/4263
