RFR(JDK12/NIO) 8202285: (fs) Add a method to Files for comparing file contents

[Joe Wang]

Hi Daniel,

You're right, line 115 is not needed. I'll add a couple of test cases to 
cover the route.

Best,
Joe

On 9/20/18, 2:36 AM, Daniel Fuchs wrote:
> Hi Joe,
>
> The spec reads very well to me.
>
> On the implementation side:
>
>  114             } else if (nRead1 != nRead2) {
>  115                 int len = Math.min(nRead1, nRead2);
>  116                 // there's always a mismatch when nRead1 != nRead2
>  117                 return totalRead +
>  118                     Arrays.mismatch(buffer1, 0, len, buffer2, 0, 
> len);
>
> I believe line 115 is not needed, and line 118 should be:
>
>     Arrays.mismatch(buffer1, 0, nRead1, buffer2, 0, nRead2);
>
> otherwise - if I'm not mistaken - there's the chance that
>
>     Arrays.mismatch(buffer1, 0, len, buffer2, 0, len);
>
> will return -1. Or am I missing something?
>
> If I'm right then your test is probably missing a case
> to explicitely test for that. Alternatively you could add a whitebox
> test to test FilesMismatch.mismatch(InputStream, InputStream)
> directly.
>
> best regards,
>
> -- daniel
>
>
> On 19/09/2018 19:48, Joe Wang wrote:
>> Hi,
>>
>> After much discussion and 10 iterations of reviews, this proposal has 
>> evolved from what was the original isSameContent method to a mismatch 
>> method. API-wise, a compare method was also considered as it looked 
>> like just a short step forward from mismatch, however, it was 
>> eventually dropped since there is no convincing use case comparing 
>> files lexicographically by contents. Impl-wise, extensive performance 
>> benchmarking has been done to compare a buffered reading vs memory 
>> mapping, the result was that a simple buffered reading performed 
>> better among small files, and those with the mismatched byte closer 
>> to the beginning of files. Since the proposed method's targeted files 
>> are small ones, the impl currently does a buffered reading only.
>>
>> Please review.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8202285
>>
>> specdiff: 
>> http://cr.openjdk.java.net/~joehw/jdk12/8202285/specdiff/java/nio/file/Files.html 
>>
>>
>> webrev: http://cr.openjdk.java.net/~joehw/jdk12/8202285/webrev/
>>
>> Thanks,
>> Joe
>>
>