RFR: 8322166: Files.isReadable/isWritable/isExecutable expensive when file does not exist

[Alan Bateman]

On Fri, 15 Dec 2023 22:49:33 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

> Add `isReadable`, `isWritable`, and `isExecutable` methods to `AbstractFileSystemProvider` and overrides to `UnixFileSystemProvider` and use these in the respective `Files` methods when the provider is an `AbstractFileSystemProvider`.

This looks quite good and consistent with the how the "exists" method was implemented before FileSystemProvider was extended.

src/java.base/share/classes/java/nio/file/Files.java line 79:

> 77: import java.util.stream.Stream;
> 78: import java.util.stream.StreamSupport;
> 79: import sun.nio.fs.AbstractFileSystemProvider;

Minor nit, you can probably group this with the imports of the other sun.nio classes.

src/java.base/unix/classes/sun/nio/fs/UnixFileSystem.java line 898:

> 896:                 int errno = access(source, W_OK);
> 897:                 if (errno != 0)
> 898:                     throw new UnixException(errno);

I don't think it's necessary to throw the UnixException here, should be able to use new UnixException(errno).rethrowAsIOException(source) to have it only throw the appropriate IOException for the error.

src/java.base/unix/classes/sun/nio/fs/UnixFileSystemProvider.java line 357:

> 355:                 throw new UnixException(errno);
> 356:         } catch (UnixException exc) {
> 357:             exc.rethrowAsIOException(file);

This is another case where the UnixException doesn't need to be thrown.

src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c line 1202:

> 1200:     RESTARTABLE(access(path, (int)amode), err);
> 1201: 
> 1202:     return err == -1 ? errno : 0;

Style wise I think keep the parentheses to make it a bit clearer to read.

src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c line 1207:

> 1205: 
> 1206: JNIEXPORT jboolean JNICALL
> 1207: Java_sun_nio_fs_UnixNativeDispatcher_exists0(JNIEnv* env, jclass this, jlong pathAddress) {

Shouldn't the UnixNativeDispatcher.exist method be removed too?

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

PR Review: https://git.openjdk.org/jdk/pull/17133#pullrequestreview-1785148620
PR Review Comment: https://git.openjdk.org/jdk/pull/17133#discussion_r1428738587
PR Review Comment: https://git.openjdk.org/jdk/pull/17133#discussion_r1428739755
PR Review Comment: https://git.openjdk.org/jdk/pull/17133#discussion_r1428741082
PR Review Comment: https://git.openjdk.org/jdk/pull/17133#discussion_r1428740458
PR Review Comment: https://git.openjdk.org/jdk/pull/17133#discussion_r1428740133