On Wed, 23 Nov 2022 16:14:52 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> wrote:
>> Per Minborg has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Cleanup > > src/java.base/share/classes/jdk/internal/access/JavaNioAccess.java line 125: > >> 123: * @see #releaseScope(Buffer, MemorySessionImpl) >> 124: */ >> 125: MemorySessionImpl acquireScopeOrNull(Buffer buffer); > > I think this looks better - but naming-wise it's still a bit problematic. > This method really acquires the underlying session, not a scope. And also, > here we have `OrNull`, but the already existing `acquireSession` also has a > similar semantics w.r.t. null. > > I suggest to rename: > > * `acquireSession` -> `acquireSessionAsRunnable` > * `acquireScopeOrNull` -> `acquireSession` > * `releaseScope` -> `releaseSession` > > Also, once we have `acquire/releaseSession`, it is not clear to me that we > still need `acquireSessionAsRunnable` in the JavaNIOAccess class - it seems > like you can create the Runnable where it's required (probably IOUtil), > simply by using straight acquire/release. The name "scope" was used in anticipation of the new proposed Java 20 naming. But I can change it back to session again. We could always rename later. > src/java.base/share/classes/jdk/internal/access/JavaNioAccess.java line 164: > >> 162: int pageSize(); >> 163: >> 164: sealed interface ScopeAcquisition extends AutoCloseable { > > Is this still needed? Well spotted. ------------- PR: https://git.openjdk.org/jdk/pull/11260