RFR: 6980847: (fs) Files.copy needs to be tuned [v7]

[Alan Bateman]

On Wed, 22 Jun 2022 01:15:19 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Modify `UnixCopyFile.copyFile()` to set the transfer size to the least common multiple of the source and destination file block sizes if the block sizes are not equal. Modify `WindowsFileCopy.copy()` to set `COPY_FILE_NO_BUFFERING` in the flags passed to `CopyFileEx()` if the size of the transfer is greater than a threshold.
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   6980847: Cleanup

04d75022 is a big improvement, I think the overall approach is good now.

src/java.base/unix/classes/sun/nio/fs/UnixCopyFile.java line 233:

> 231:     private static long lcm(long x, long y) {
> 232:         if (x <= 0 || y <= 0)
> 233:             throw new IllegalArgumentException("Non-positive parameter");

If lcm were to be used more widely then throwing IAE would make sense. However, the usage here is private to the copy method and it only calls lcm when x and y are positive. It also catches IAE. It might be simpler to just put an assert x > 0 && y > 0 and remove the catch of IAE. That would catch any bugs in this code.

src/java.base/unix/classes/sun/nio/fs/UnixCopyFile.java line 330:

> 328:                     } finally {
> 329:                         Util.releaseTemporaryDirectBuffer(buf);
> 330:                         Blocker.end(comp);

It might be better to use a nested try-finally to ensure that the buffer get/release and blocker begin/end are symmetric.

src/java.base/unix/classes/sun/nio/fs/UnixCopyFile.java line 733:

> 731:      *        (a non-zero value written to this address indicates cancel)
> 732:      */
> 733:     private static native void bufferCopy0(int dst, int src, long address,

It might be clearer if the name were bufferedCopy rather than bufferCopy.

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

PR: https://git.openjdk.org/jdk/pull/9161