RFR: JDK-8273038: ChannelInputStream.transferTo() uses FileChannel.transferTo(FileChannel) [v12]

[Alan Bateman]

On Mon, 6 Sep 2021 10:08:28 GMT, Markus KARG <github.com+1701815+mkarg at openjdk.org> wrote:

>> As proposed in JDK-8265891, this PR provides an implementation for `Channels.newInputStream().transferTo()` which provide superior performance compared to the current implementation. This PR is a spin-off from https://github.com/openjdk/jdk/pull/4263 and https://github.com/openjdk/jdk/pull/5179 and deliberately concentrates **only** on the case where both, source **and** target, are actually `FileChannel`s. Other types of channels will be discussed in future PRs.
>> 
>> * 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 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. A rather similar approach in different use casse was recently implemented by https://github.com/openjdk/jdk/pull/5097 which was reported by even provide 5x performance (https://github.com/openjdk/jdk/pull/5097#issuecomment-897271997).
>
> Markus KARG has updated the pull request incrementally with five additional commits since the last revision:
> 
>  - Replaced 'point' by 'position' in comments
>    
>    Signed-off-by: Markus Karg <markus at headcrashing.eu>
>  - Moved TransferTo.java from test/jdk/sun/nio/ch/ChannelInputStream/ to test/jdk/java/nio/channels/Channels/
>    
>    Signed-off-by: Markus Karg <markus at headcrashing.eu>
>  - Redeced line lenght; maximum length now is 115 characters
>    
>    Signed-off-by: Markus Karg <markus at headcrashing.eu>
>  - Removed static modifiers on interfaces
>    
>    Signed-off-by: Markus Karg <markus at headcrashing.eu>
>  - Removed unused imports
>    
>    Signed-off-by: Markus Karg <markus at headcrashing.eu>

> Agreed that _at some point_ (i. e. in _some_ future PR) someone could provide such a test. As explained in [#5209 (comment)](https://github.com/openjdk/jdk/pull/5209#issuecomment-907782217) there is no 100% path coverage rule in OpenJDK, so while this _might_ be a good idea, it seems it is definitively not a _mandatory_ constraint to test that loop at all.

The override of InputStream::transferTo is calling FileChannel::transferTo in a loop so I don't think it is unreasonable to ask if we have test that has more than one iteration. I'm okay if we defer this to a further step but I do think we should have test for this case.


> It actually reads "2014, 2021" which means that the original code was first published in 2014 and was last modified in 2021. Oracle once asked me to always do it _exactly that way_ when touching any Oracle source code (which I did here, as the test originated from existing Oracle source code). If OpenJDK is an exceptional case here, then please confirm that I am _officially allowed_ to remove the year 2014 from that list. Until then I have to keep the line exactly the way it is to not get into any troubles with Oracle's legal department.

This is a new test, the date should be 2021.

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

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