On Mon, 21 Nov 2022 14:07:38 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Per Minborg has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Rename methods > > src/java.base/share/classes/java/util/zip/Adler32.java line 105: > >> 103: adler = updateByteBuffer(adler, >> ((DirectBuffer)buffer).address(), pos, rem); >> 104: } finally { >> 105: Reference.reachabilityFence(buffer); > > The updated naming is a bit better but there are it still feels that that > there are too many things going on at the use sites ("guard", "session", > "reachability fence"). Ideally the acquire would return something with an > accessor for the direct buffer address but that may not be possible. I > wonder if changing NOP_CLOSE.close to do reachabilityFence(this) would work > as expected and improve many of these use sites? This can certainly be done. We could, for example, add a new method to the `JavaNioAccess` interface: ```AddressAcquisition acquireSession2(Buffer buffer); // to be renamed``` This would allow us to go from: try (var guard = NIO_ACCESS.acquireSession(buffer)) { adler = updateByteBuffer(adler, ((DirectBuffer)buffer).address(), pos, rem); } finally { Reference.reachabilityFence(buffer); } to try (var guard = NIO_ACCESS.acquireSession2(buffer)) { adler = updateByteBuffer(adler, guard.address(), pos, rem); } Although this looks much simpler and concise, it means "a new object is created for each invocation" (*). This also allows us to remove the `@SupressWarnings("try")` annotations. Here is how the undercarriage might look like: @Override public AddressAcquisition acquireSession2(Buffer buffer) { var session = buffer.session(); if (session == null) { return AddressAcquisition.create(buffer, () -> {}); } session.acquire0(); return AddressAcquisition.create(buffer, session::release0); } and sealed interface AddressAcquisition extends AutoCloseable { long address(); @Override void close(); static AddressAcquisition create(Buffer buffer, Runnable closeActions) { return new AddressAcquisitionImpl(buffer, closeActions); } final class AddressAcquisitionImpl implements AddressAcquisition { private final DirectBuffer directBuffer; private final Runnable closeAction; public AddressAcquisitionImpl(Buffer buffer, Runnable closeAction) { this.directBuffer = (DirectBuffer) buffer; this.closeAction = closeAction; } @Override public long address() { return directBuffer.address(); } @Override public void close() { try { closeAction.run(); } finally { Reference.reachabilityFence(directBuffer); } } } } (*) In reality, a representation of the object might be allocated on the stack instead. ------------- PR: https://git.openjdk.org/jdk/pull/11260