On Tue, 20 May 2025 12:54:06 GMT, Per Minborg <pminb...@openjdk.org> wrote:
>> This PR proposes to use `JavaNioAccess::getBufferAdress` rather than >> `DirectBuffer::address` so that `Buffer` instances backed by MemorySegment >> instances can be used in classes that were not covered by >> https://github.com/openjdk/jdk/pull/25321 >> >> This PR passes tier1, tier2, and tier3 tests on multiple platforms and >> configurations. > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Clean up src/java.base/share/classes/sun/nio/ch/Util.java line 46: > 44: public class Util { > 45: > 46: static final JavaNioAccess NIO_ACCESS = > SharedSecrets.getJavaNioAccess(); There are methods in IOUtil for the classes in sun.nio.ch. So no need to expose NIO_ACCESS here. src/java.base/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java line 671: > 669: > 670: } finally { > 671: NIO_ACCESS.releaseSession(buf); I thin the change to WindowsAsynchronousFileChannel will need more than one reviewer as the I/O operation may complete on a different thread to the one that it was initiated on. src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11AEADCipher.java line 764: > 762: int outOfs = 0; > 763: if (outBuffer instanceof DirectBuffer) { > 764: outAddr = NIO_ACCESS.getBufferAddress(outBuffer); This looks right. src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line 1017: > 1015: int outOfs = 0; > 1016: if (outBuffer instanceof DirectBuffer) { > 1017: outAddr = NIO_ACCESS.getBufferAddress(outBuffer); This looks right. src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Digest.java line 292: > 290: NIO_ACCESS.acquireSession(byteBuffer); > 291: try { > 292: token.p11.C_DigestUpdate(session.id(), > NIO_ACCESS.getBufferAddress(byteBuffer) + ofs, null, 0, len); `long address = NIO_ACCESS.getBufferAddress(byteBuffer);` might be clearer here. src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Mac.java line 289: > 287: NIO_ACCESS.acquireSession(byteBuffer); > 288: try { > 289: token.p11.C_SignUpdate(session.id(), > NIO_ACCESS.getBufferAddress(byteBuffer) + ofs, null, 0, len); I think I would introduce a long address here too, only to make the line easier to read. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25324#discussion_r2098279301 PR Review Comment: https://git.openjdk.org/jdk/pull/25324#discussion_r2098297686 PR Review Comment: https://git.openjdk.org/jdk/pull/25324#discussion_r2098298906 PR Review Comment: https://git.openjdk.org/jdk/pull/25324#discussion_r2098299247 PR Review Comment: https://git.openjdk.org/jdk/pull/25324#discussion_r2098300469 PR Review Comment: https://git.openjdk.org/jdk/pull/25324#discussion_r2098302426
