On Sun, 30 May 2021 17:30:56 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?

Adding an override of transferTo may require new tests. @bplb Do you if we have 
good tests for all the scenarios where input stream returned by 
Channels.newInputStream is the source?

I think we also need to decide if this PR is about Channels.newInputStream or 
the input stream return by Files.newInputStream as the latter could return its 
own input stream implementation, it doesn't need to ChannelInputStream.

If the approach on the table goes ahead then ChannelInputStream.transferTo can 
be split into 4 simple methods to make it easier to maintain. You can use 
something like "total" or "bytesWritten" for the total number of bytes written 
rather than "i".

-------------

PR: https://git.openjdk.java.net/jdk/pull/4263

Reply via email to