On Mon, 26 Apr 2021 17:10:13 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> wrote:
> This PR contains the API and implementation changes for JEP-412 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/412 src/java.base/share/classes/java/lang/Module.java line 253: > 251: } > 252: > 253: boolean isEnableNativeAccess() { Would it be possible change isEnableNativeAccess and addNEnableNativeAccess to keep them consistent with the existing methods in this file. src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java line 384: > 382: Module addEnableNativeAccess(Module m); > 383: > 384: boolean isEnableNativeAccess(Module m); Probably better to co-locate these with the other module methods, and add a comment so that it's consistent with the existing methods. src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 272: > 270: BootLoader.loadModule(base); > 271: SharedSecrets.getJavaLangAccess() > 272: .addEnableNativeAccess(Modules.defineModule(null, > base.descriptor(), baseUri)); This would be cleaner if you replace it with: Module baseModule = Modules.defineModule(null, base.descriptor(), baseUri); JLA.addEnableNativeAccess(baseModule); You can use JLA in addEnableNativeAccess too, no need for "jla". src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 876: > 874: } > 875: > 876: private static void addEnableNativeAccess(ModuleLayer layer) { It would be useful to add a method description in the same style as the existing methods, just to keep these methods consistent. src/java.base/share/classes/sun/nio/ch/IOUtil.java line 57: > 55: > 56: static int write(FileDescriptor fd, ByteBuffer src, long position, > 57: boolean async, NativeDispatcher nd) UnixAsynchronousSocketChannel is an outlier user of IOUtil and I think we need to see if we can avoid extending this class to async if possible. src/java.base/share/classes/sun/nio/ch/IOUtil.java line 466: > 464: } > 465: > 466: private static final JavaNioAccess NIO_ACCESS = > SharedSecrets.getJavaNioAccess(); It might be cleaner to move to acquire/release methods to their own supporting class as it's not really IOUtil. ------------- PR: https://git.openjdk.java.net/jdk/pull/3699
