On Thu, 31 Oct 2024 15:52:04 GMT, Quan Anh Mai <qa...@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
