On Mon, 21 Nov 2022 14:07:38 GMT, Alan Bateman <[email protected]> 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
