RFR: 8343394: Make MemorySessionImpl.state a stable field

[Maurizio Cimadamore]

On Thu, 31 Oct 2024 15:52:04 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> Hi,
> 
> This patch makes `MemorySessionImpl.state` a `Stable` field so that liveness check of non-closeable scopes such as the global scope can be elided.
> 
> Currently, the `state` field is overloaded with 2 responsibilities, to act as a communication device between `close` and `checkValidState`, as well as a communication device between `close`, `acquire`, and `release`. This patch separates those concerns into `state` and `acquireCount`, allowing `state` to be marked as `@Stable`.
> 
> With the patch, in `MemorySegmentGetUnsafe`, `panama` is able to be on par with `unsafe`:
> 
>     Benchmark                      Mode  Cnt  Score   Error  Units
>     MemorySegmentGetUnsafe.panama  avgt   30  0.340 ± 0.008  ns/op
>     MemorySegmentGetUnsafe.unsafe  avgt   30  0.332 ± 0.004  ns/op
> 
> For reference this is the results without this patch:
> 
>     Benchmark                      Mode  Cnt  Score   Error  Units
>     MemorySegmentGetUnsafe.panama  avgt   30  0.420 ± 0.019  ns/op
>     MemorySegmentGetUnsafe.unsafe  avgt   30  0.329 ± 0.003  ns/op
> 
> Please kindly review, thanks very much.

src/java.base/share/classes/jdk/internal/foreign/MemorySessionImpl.java line 61:

> 59:     static final int OPEN = 0;
> 60:     static final int CLOSED = -1;
> 61:     static final int NONCLOSEABLE = 1;

Nice trick! This effectively relegates 0 (open) to be the default value of the stable state field. That will either go to -1 (closed) or 1 (non-closeable) and be treated as constant thereafter. Perhaps worth adding a comment on the rationale behind this.

src/java.base/share/classes/jdk/internal/foreign/SharedSession.java line 90:

> 88:         }
> 89: 
> 90:         state = CLOSED;

I like it that we still only have one CAS here (as only one thread can set CLOSED_ACQUIRE_COUNT). So shared arena close doesn't need more work. You might want to check the MemorySessionClose bench, just in case.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21810#discussion_r1824848427
PR Review Comment: https://git.openjdk.org/jdk/pull/21810#discussion_r1824847100