Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-06 Thread Maurizio Cimadamore
On Tue, 5 Nov 2024 14:07:12 GMT, Quan Anh Mai 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 a

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v3]

2024-11-06 Thread Quan Anh Mai
On Thu, 31 Oct 2024 23:08:40 GMT, Maurizio Cimadamore wrote: >> Quan Anh Mai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix benchmarks > > Actually, @JornVernee pointed out (great observation!) that this PR creates a > new asymmet

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-06 Thread Per Minborg
On Tue, 5 Nov 2024 18:48:07 GMT, Quan Anh Mai wrote: >> Quan Anh Mai has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - copyright year >> - address reviews > > Thanks a lot for your reviews and suggestions, I will merge this patch. Than

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Quan Anh Mai
On Tue, 5 Nov 2024 14:07:12 GMT, Quan Anh Mai 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 a

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Quan Anh Mai
On Tue, 5 Nov 2024 15:31:11 GMT, Maurizio Cimadamore wrote: >> Okay, fair enough. > > Using two blocking queues might also be possible: > * at the start of each iteration a thread does a `get` on its own queue > * at the end of each iteration, a thread does a put on the other thread's > queue >

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Quan Anh Mai
On Tue, 5 Nov 2024 15:29:22 GMT, Per Minborg wrote: >> Quan Anh Mai has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - copyright year >> - address reviews > > In `SharedSession`, would it not be necessary to override `isClosable()` so >

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Maurizio Cimadamore
On Tue, 5 Nov 2024 15:13:29 GMT, Jorn Vernee wrote: >> I'm afraid that it would be too expensive compared to a spinlock, and the >> scheduler might not wake up the threads soon enough to make a meaningful >> race, greatly reducing the effectiveness of the test. > > Okay, fair enough. Using two

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Per Minborg
On Tue, 5 Nov 2024 14:07:12 GMT, Quan Anh Mai 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 a

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Jorn Vernee
On Tue, 5 Nov 2024 14:49:09 GMT, Quan Anh Mai wrote: >> test/jdk/java/foreign/TestMemorySession.java line 374: >> >>> 372: Thread.onSpinWait(); >>> 373: k = lock.get(); >>> 374: } >> >> I think the right primitive here is a `CyclicBarrier`

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Jorn Vernee
On Tue, 5 Nov 2024 14:07:12 GMT, Quan Anh Mai 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 a

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Quan Anh Mai
On Tue, 5 Nov 2024 14:43:29 GMT, Jorn Vernee wrote: >> Quan Anh Mai has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - copyright year >> - address reviews > > test/jdk/java/foreign/TestMemorySession.java line 374: > >> 372:

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Jorn Vernee
On Tue, 5 Nov 2024 14:07:12 GMT, Quan Anh Mai 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 a

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-05 Thread Quan Anh Mai
On Fri, 1 Nov 2024 02:44:15 GMT, Quan Anh Mai 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 a

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v5]

2024-11-05 Thread Quan Anh Mai
> 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 `checkVal

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-05 Thread Quan Anh Mai
On Mon, 4 Nov 2024 19:08:49 GMT, Jorn Vernee wrote: >> `checkValidStateRaw` synchronizes with `justClose` using handshakes so an >> opaque or higher load is only needed in `sharedSessionAlreadyClosed`. A >> `getOpaque` would probably be adequate. But I believe there is no formal >> restriction

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-05 Thread Per Minborg
On Mon, 4 Nov 2024 21:57:39 GMT, ExE Boss wrote: >> Doesn't using `setOpaque` mean that another thread may see the update to >> `state` before the update to `acquireCount`? i.e. the scope of a memory >> segment may appear closed, but the segment would still be passable to a >> downcall? > > `a

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-04 Thread ExE Boss
On Mon, 4 Nov 2024 19:08:49 GMT, Jorn Vernee wrote: >> `checkValidStateRaw` synchronizes with `justClose` using handshakes so an >> opaque or higher load is only needed in `sharedSessionAlreadyClosed`. A >> `getOpaque` would probably be adequate. But I believe there is no formal >> restriction

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-04 Thread Jorn Vernee
On Mon, 4 Nov 2024 13:17:23 GMT, Quan Anh Mai wrote: >>> I'm dubious about this. >> >> NVM, I see it now -`sharedSessionAlreadyAcquired` uses a `getVolatile` >> (which has stronger semantics), so we're in the clear. > > `checkValidStateRaw` synchronizes with `justClose` using handshakes so an

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-04 Thread Jorn Vernee
On Fri, 1 Nov 2024 02:44:15 GMT, Quan Anh Mai 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 a

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-04 Thread Quan Anh Mai
On Mon, 4 Nov 2024 11:32:12 GMT, Maurizio Cimadamore wrote: >> I'm dubious about this. Typically these modes work in a symmetric way - if >> you `setOpaque` you need to `getOpaque`. From >> [this](https://gee.cs.oswego.edu/dl/html/j9mm.html): >> >>> Opaque mode, obtained using VarHandle getOp

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-04 Thread Maurizio Cimadamore
On Mon, 4 Nov 2024 10:12:47 GMT, Maurizio Cimadamore wrote: > I'm dubious about this. NVM, I see it now -`sharedSessionAlreadyAcquired` uses a `getVolatile` (which has stronger semantics), so we're in the clear. - PR Review Comment: https://git.openjdk.org/jdk/pull/21810#discuss

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-04 Thread Maurizio Cimadamore
On Fri, 1 Nov 2024 10:05:22 GMT, Quan Anh Mai wrote: >> src/java.base/share/classes/jdk/internal/foreign/SharedSession.java line 90: >> >>> 88: } >>> 89: >>> 90: STATE.setOpaque(this, CLOSED); >> >> Why are we using opaque semantics here and not volatile? > > Because this varia

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-01 Thread Quan Anh Mai
On Fri, 1 Nov 2024 08:55:09 GMT, Per Minborg wrote: >> Quan Anh Mai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> wait for the close operation to complete on acquire failures > > src/java.base/share/classes/jdk/internal/foreign/SharedS

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-11-01 Thread Per Minborg
On Fri, 1 Nov 2024 02:44:15 GMT, Quan Anh Mai 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 a

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v4]

2024-10-31 Thread Quan Anh Mai
> 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 `checkVal

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v3]

2024-10-31 Thread Quan Anh Mai
On Thu, 31 Oct 2024 18:09:55 GMT, Quan Anh Mai 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

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v3]

2024-10-31 Thread Maurizio Cimadamore
On Thu, 31 Oct 2024 18:09:55 GMT, Quan Anh Mai 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

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v2]

2024-10-31 Thread Maurizio Cimadamore
On Thu, 31 Oct 2024 18:06:22 GMT, Quan Anh Mai wrote: >> src/java.base/share/classes/jdk/internal/foreign/ConfinedSession.java line >> 44: >> >>> 42: private int asyncReleaseCount = 0; >>> 43: >>> 44: static final VarHandle ASYNC_RELEASE_COUNT= >>> MhUtil.findVarHandle(MethodHandles.l

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v3]

2024-10-31 Thread Quan Anh Mai
On Thu, 31 Oct 2024 17:03:31 GMT, Maurizio Cimadamore wrote: >> Quan Anh Mai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix benchmarks > > src/java.base/share/classes/jdk/internal/foreign/SharedSession.java line 90: > >> 88:

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v2]

2024-10-31 Thread Quan Anh Mai
On Thu, 31 Oct 2024 17:41:22 GMT, Maurizio Cimadamore wrote: >> Quan Anh Mai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> add LoopOverRandom benchmarks > > src/java.base/share/classes/jdk/internal/foreign/ConfinedSession.java line 44

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v2]

2024-10-31 Thread Quan Anh Mai
On Thu, 31 Oct 2024 17:23:51 GMT, Quan Anh Mai 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

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v3]

2024-10-31 Thread Quan Anh Mai
> 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 `checkVal

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v2]

2024-10-31 Thread Maurizio Cimadamore
On Thu, 31 Oct 2024 17:23:51 GMT, Quan Anh Mai 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

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v2]

2024-10-31 Thread Maurizio Cimadamore
On Thu, 31 Oct 2024 17:24:23 GMT, Quan Anh Mai wrote: > For some reasons, the compiler really dislikes the unsafe loop. On my slower machine, the difference between the variants is bigger: BenchmarkMode Cnt Score Error Units LoopOverRandom.segment_loop

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v2]

2024-10-31 Thread Maurizio Cimadamore
On Thu, 31 Oct 2024 17:23:51 GMT, Quan Anh Mai 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

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v2]

2024-10-31 Thread Quan Anh Mai
On Thu, 31 Oct 2024 17:23:51 GMT, Quan Anh Mai 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

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field [v2]

2024-10-31 Thread Quan Anh Mai
> 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 `checkVal

Re: RFR: 8343394: Make MemorySessionImpl.state a stable field

2024-10-31 Thread Maurizio Cimadamore
On Thu, 31 Oct 2024 15:52:04 GMT, Quan Anh Mai 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

RFR: 8343394: Make MemorySessionImpl.state a stable field

2024-10-31 Thread Quan Anh Mai
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 w