RFR: JDK-8273038: ChannelInputStream.transferTo() uses FileChannel.transferTo(FileChannel) [v4]
Markus KARG
github.com+1701815+mkarg at openjdk.java.net
Sun Aug 29 12:18:28 UTC 2021
On Wed, 25 Aug 2021 21:37:28 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:
>> Markus KARG has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Simplified copy loop
>>
>> Signed-off-by: Markus Karg <markus at headcrashing.eu>
>
> Why not just use [java/io/FileInputStream/TransferTo.java](https://github.com/openjdk/jdk/blob/master/test/jdk/java/io/FileInputStream/TransferTo.java) as a starting point?
@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 li
ve 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.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5209
More information about the nio-dev
mailing list