RFR: 8265891: ChannelInputStream.transferTo() uses FileChannel.transferTo(FileChannel)

Markus KARG github.com+1701815+mkarg at openjdk.java.net
Tue Aug 24 20:40:32 UTC 2021


On Tue, 24 Aug 2021 12:17:18 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> There are 3 PRs open, I'll ignore #4263 and #5179 for now.

Yes please ignore them for now. The other two PRs are suspendend as they cover bigger pictures and will be resumed by me *later*.

> The moving of the OutputStream implementation from Channels to sun.nio.ch.ChannelOutputStream looks fine. In the class description it should be "accessible" as it visible. You can drop "but not be part of the java.base module API" too.

Done in https://github.com/openjdk/jdk/pull/5209/commits/f35cd7d74f2ac15f60e7d97395ec71deefeafb76.

> It would be good if you could re-format 1-arg transferTo to something like this:
> ...
> just to to avoid the really long line as that gets annoying when looking at side-by-side diffs.

Never had this problem as I'm using inline-diff tools only, but done in https://github.com/openjdk/jdk/pull/5209/commits/33bef3d6ef33bb9eddc66e69c1a1e6fd9d565f34.

> An alternative, that avoids the need for "srcPos + bytesWritten" in 3 places is to just something like:
> ...

Does that mean that I must change it? As the method is mostly I/O bound I do not see what we actually would gain.

> I suspect it would be a lot cleaner/maintainable if it were written as a TestNG test. I assume the transfer loop never has more than one iteration because the file sizes are <2GB. If it were writing this test then I would probably use Files/Arrays.mismatch to compare the contents.

Understood, but please note that this code actually was simply copied from the original tests that Brian provided as a blue-print, which neither uses TestNG nor Files/Arrays.mismatch nor tests larger files. So if you ask me to change that, in turn someone should change the original tests then to keep the code in sync. So can we keep the tests as-is or am I forced to use TestNG and File/Arrays.mismatch?

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

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


More information about the nio-dev mailing list