RFR: 8357268: Use JavaNioAccess.getBufferAddress rather than DirectBuffer.address() [v3]
Alan Bateman
alanb at openjdk.org
Tue May 20 15:43:00 UTC 2025
On Tue, 20 May 2025 12:54:06 GMT, Per Minborg <pminborg at 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
More information about the net-dev
mailing list