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

[Daniel Fuchs]

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
>