RFR: 8310426: (ch) Channels.newInputStream transferTo cleanup

Markus KARG duke at openjdk.org
Tue Jun 20 21:48:02 UTC 2023


On Tue, 20 Jun 2023 15:15:51 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> ChannelInputStream, the InputStream returned by Channels.newInputStream, implements the transferTo method with support for the cross product of many interesting underlying channels. It's getting a bit too unwieldy and can be simplified by introducing a FileChannelInputStream to wrap a FileChannel.

The undoubted benefits (pros) of this PR is:
* A cleaner ChannelInputStream::transferTo implementation.

The obvious drawbacks (contras) of this PR are:
* The overall optimization solution ("the sum of all optimizations") is much harder to grasp now (it took me half an hour just to check if we still cover the full cross product), as the solution is scattered over lots of files now. In contrast, a central place was much simpler to grasp before this PR.
* IIUC the PR needed to duplicate algorithms just for the sole sake of code cleanup. In contrast, a central place was the sole source of truth for all transfer algorithms before this PR.
* The new solution is much more complex and comes with additional classes just for the sole sake of a method's cleanup; those classes apparently have no other benefit, at least so far.

IMHO the single benefit does not outweigh the multiple drawbacks.

A counter proposal for a compromise between this PR (code duplication) and the original code (single source of truth) could be:
* Let's refactor the cross product (the original trigger for this PR) on the channel level (not on the stream level) into a new static utility method Channels::transfer(ReadableByteChannel, WritableByteChannel), and let's call that single method from all code locations where "some channels" need to transfer bytes.

The benefit of the counter proposal would be:
* Keeping a single source of truth, which does not need code duplication, and which is easy to grasp (all optimization rules in one place).
* Can be used with either the original code and/or the new PR.
* Can simply get centrally extended in case a future OS comes up with a new type of optimization API eventually, e. g. hardware-implemented Socket-to-Socket transfers.
* Could even get used by other JDK locations, and/or custom application code that wants to perform optimized non-stream channel-to-channel transfers.

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

PR Review: https://git.openjdk.org/jdk/pull/14566#pullrequestreview-1489148545


More information about the nio-dev mailing list