[foreign-memaccess] [Rev 02] RFR: Alternative scalable MemoryScope

[Maurizio Cimadamore]

On Tue, 5 May 2020 18:48:43 GMT, Peter Levart <plevart at openjdk.org> wrote:

>> This is an alternative MemoryScope which is more scalable when used in a scenario where child scope is frequently
>> acquired and closed concurrently from multiple threads (for example in parallel Stream.findAny())
>
> Peter Levart has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Re-add fullFence() in MemorySegment.withOwnerThread() due to effect on the segment backing memory which is not
>   guaranteed by JMM otherwise

The code looks good - leaning towards approving and see how it goes.

One thing I noted is that there are many assumptions that some methods can only be called from owner thread - but while
that's true for the 'safe' API, that's not the case for the unsafe API; in that case I have a vague feeling that the
old atomic would behave a bit more predictably.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/MemoryScope.java line 46:

> 45:     /**
> 46:      * Creates a root MemoryScope with given ref and cleanupAction.
> 47:      * The returned instance may be published unsafely to and used in any thread, but methods that explicitly state
> that

While this is true, note that it is possible to unsafely create unconfined segments, in which case the ownership
restriction would not apply. Perhaps the doc should be tweaked a bit to reflect this.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/MemoryScope.java line 91:

> 90:      * Closes this scope, executing any cleanup action if this is the root scope.
> 91:      * This method may only be called in "owner" thread.
> 92:      *

Again - for unsafe segments this is not true. Not that we need to provide guarantees in that case - but I think it's
better to make things clear.

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

Marked as reviewed by mcimadamore (Committer).

PR: https://git.openjdk.java.net/panama-foreign/pull/142