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

[Markus KARG]

On Sun, 29 Aug 2021 16:24:01 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> @bplb @AlanBateman As discussed before, I have changed the test in this PR to cover the following cases:
>> * Testing random buffer sizes
>> * Testing random initial source position
>> * Testing random initial target position
>> 
>> I deliberately did not test the following:
>> * 2GB+: IMHO makes not much sense to add such a heavy weight test as the expected benefit will not really outweigh the test complexity needed, will slow down test performance massively, and was not tested in the original implementation / in https://github.com/openjdk/jdk/blob/master/test/jdk/java/io/FileInputStream/TransferTo.java, too. There is no rule that we have to provide 100% test coverage in OpenJDK. Actually adding a test must be well justified and is not the only measure we have in the box to prevent the occurance of bugs in production: I suggest, instead of a 2GB+ test, we agree that the contributed implementation *looks* correct to each of us (= applying six-eyes principle). For such a simple code line, this should be enough QA, as in case it unexpectedly would break, it is very likely to get reported by beta testers long before GA (possibly by myself as many of my projects move around 4GB+ sized files in reality), which should be early enough to catch and fix.
>> * Same for testing position-after-transfer. This is not covered by the original test, and due to our six eyes it is highly unlikely that it is broken or will break in future. So I'd say KISS here.
>> 
>> Wherever it made sense (without sacrifying my intended modularity needed for other channel types tested later) I tried to follow the solutions found in https://github.com/openjdk/jdk/blob/master/test/jdk/java/io/FileInputStream/TransferTo.java. Thanks Bran, for this blue print. In fact I wonder if the introduction of 10 loops over random sizes actually will improve test qualify. My assumption is that it rather reduces test stability and test repeatability: In case there actually is a bug that happens only with a specific combination of buffer size and initial positions, ten iterations will be much too few to stably reproduce the bug at each execution of the test. More often that not, that ten loops will all pass without running into the problem given the huge max size. This will driver testers nuts! The improved entropy by introducing the JDK randomizer does not help much to improve this. Having said that, I copied the solution from Brian only because he came up with it, and I can
  live with the statistical unpredictability it induces, but just wanted to clearly say that I (spoiler: working for a company providing QA tools like FMEA and SPC software) would say introducing random here induces more harm due to non-repeatability than it actualy improves the detection rate of failures not detected by thorough six-eye review.
>
>> * 2GB+: IMHO makes not much sense to add such a heavy weight test as the expected benefit will not really outweigh the test complexity needed, 
> 
> I'll try to get time to look at what you have in the next few days but just to say that the suggestion for exercising files > 2GB is because it's beyond the limit of the underlying sendfile. You should be able to check this quickly by printing the iteration count in ChannelInputStream.transferTo, it is always 1?

@AlanBateman @bplb Would be cool if we could finish this PR this week, so if there are serious pain points that MUST get covered it would be great if you find the time to clearly point them out in the next days! Thanks! :-)

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

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