On Wed, 23 Nov 2022 16:14:52 GMT, Maurizio Cimadamore <[email protected]>
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
