[foreign-memaccess+abi] RFR: 8293400: Modify static constructors in MemorySegment [v2]

[Maurizio Cimadamore]

On Tue, 13 Sep 2022 11:32:10 GMT, Per Minborg <duke at openjdk.org> wrote:

>> This PR changes the `MemorySegment` static constructors to be more like the one of `ByteBuffer`.
>
> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Simplify creatons of MemorySegments in tests

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 62:

> 60:  * which falls <em>outside</em> the boundaries of the memory segment being accessed. Temporal bounds ensure that memory access
> 61:  * operations on a segment cannot occur after the memory session associated with a memory segment has been closed (see {@link MemorySession#close()}).
> 62:  *

I note that the package-info has not been updated. That probably contains samples that no longer work. Please look for all occurrences of `allocateNative` in javadoc samples, and make sure they are correct

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 957:

> 955:      * The memory segment is automatically freed some unspecified time after it is no longer referenced.
> 956:      * If a memory segment needs to be deterministically freed, use an appropriate memory session from which
> 957:      * {@linkplain MemorySession#allocate(MemoryLayout) allocation} can be made instead.

Suggestion:

     * The returned memory segment is associated with an {@linkplain MemorySession#openImplicit implicit} memory session. As such, the native memory region associated with the returned segment is freed <em>automatically</em>, some unspecified time after it is no longer referenced. Native segments featuring deterministic deallocation can be obtained using the {MemorySession#allocate} method.

src/java.base/share/classes/java/lang/foreign/MemorySession.java line 222:

> 220:      * segment is no longer in use. Failure to do so will result in off-heap memory leaks.
> 221:      * <p>
> 222:      * The block of off-heap memory associated with the returned native memory segment is initialized to zero.

Suggestion:

     * The off-heap memory associated with the returned native memory segment is initialized to zero.

src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line 466:

> 464:         final class Holder {
> 465:             static final SegmentAllocator IMPLICIT_ALLOCATOR = (size, align) ->
> 466:                     MemorySession.openImplicit().allocate(size, align);

Can't this use `allocateNative` ?

test/jdk/java/foreign/TestByteBuffer.java line 764:

> 762:     public static Object[][] segments() throws Throwable {
> 763:         return new Object[][] {
> 764:                 { (Supplier<MemorySegment>) () -> MemorySession.openImplicit().allocate(16) },

This could use `allocateNative`

test/jdk/jdk/incubator/vector/Byte128VectorLoadStoreTests.java line 482:

> 480:     @Test(dataProvider = "byteByteProviderForIOOBE")
> 481:     static void loadMemorySegmentIOOBE(IntFunction<byte[]> fa, IntFunction<Integer> fi) {
> 482:         MemorySegment a = toSegment(fa.apply(SPECIES.length()), i -> MemorySession.openImplicit().allocate(i, Byte.SIZE));

allocateNative here

test/jdk/jdk/incubator/vector/Byte128VectorLoadStoreTests.java line 511:

> 509:     @Test(dataProvider = "byteByteProviderForIOOBE")
> 510:     static void storeMemorySegmentIOOBE(IntFunction<byte[]> fa, IntFunction<Integer> fi) {
> 511:         MemorySegment a = toSegment(fa.apply(SPECIES.length()), i -> MemorySession.openImplicit().allocate(i, Byte.SIZE));

allocateNative here

test/jdk/jdk/incubator/vector/Byte128VectorLoadStoreTests.java line 512:

> 510:     static void storeMemorySegmentIOOBE(IntFunction<byte[]> fa, IntFunction<Integer> fi) {
> 511:         MemorySegment a = toSegment(fa.apply(SPECIES.length()), i -> MemorySession.openImplicit().allocate(i, Byte.SIZE));
> 512:         MemorySegment r = MemorySession.openImplicit().allocate(a.byteSize(), Byte.SIZE);

and here (please check the whole file)

test/jdk/jdk/incubator/vector/Byte256VectorLoadStoreTests.java line 482:

> 480:     @Test(dataProvider = "byteByteProviderForIOOBE")
> 481:     static void loadMemorySegmentIOOBE(IntFunction<byte[]> fa, IntFunction<Integer> fi) {
> 482:         MemorySegment a = toSegment(fa.apply(SPECIES.length()), i -> MemorySession.openImplicit().allocate(i, Byte.SIZE));

This file also seems like it could  benefit from using `MemorySegment.allocateNative`

test/jdk/jdk/incubator/vector/Byte512VectorLoadStoreTests.java line 482:

> 480:     @Test(dataProvider = "byteByteProviderForIOOBE")
> 481:     static void loadMemorySegmentIOOBE(IntFunction<byte[]> fa, IntFunction<Integer> fi) {
> 482:         MemorySegment a = toSegment(fa.apply(SPECIES.length()), i -> MemorySession.openImplicit().allocate(i, Byte.SIZE));

And this file too. Please check all the vector benchmark files.

test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadHelper.java line 86:

> 84:     static final MemorySegment confinedPoint = MemorySession.openConfined().allocate(POINT_LAYOUT);
> 85: 
> 86:     static final MemorySegment point = MemorySession.openImplicit().allocate(POINT_LAYOUT);

Allocate native in here and in the line below

test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNew.java line 136:

> 134:     public void segment_loop_implicit() {
> 135:         if (gcCount++ == 0) System.gc(); // GC when we overflow
> 136:         MemorySegment segment = MemorySession.openImplicit().allocate(ALLOC_SIZE, 4);

allocateNative here

test/micro/org/openjdk/bench/java/lang/foreign/MemorySessionClose.java line 121:

> 119:     @Benchmark
> 120:     public MemorySegment implicit_close() {
> 121:         return MemorySession.openImplicit().allocate(ALLOC_SIZE, 4);

allocateNative here

test/micro/org/openjdk/bench/java/lang/foreign/MemorySessionClose.java line 127:

> 125:     public MemorySegment implicit_close_systemgc() {
> 126:         if (gcCount++ == 0) System.gc(); // GC when we overflow
> 127:         return MemorySession.openImplicit().allocate(ALLOC_SIZE, 4);

allocateNative here

test/micro/org/openjdk/bench/jdk/incubator/vector/TestLoadStoreBytes.java line 87:

> 85:     dstSegmentHeap = MemorySegment.ofArray(new byte[size]);
> 86: 
> 87:     srcSegment = MemorySession.openImplicit().allocate(size, SPECIES.vectorByteSize());

allocateNative here

test/micro/org/openjdk/bench/jdk/incubator/vector/TestLoadStoreShorts.java line 92:

> 90:     dstSegmentHeap = MemorySegment.ofArray(new byte[size]);
> 91: 
> 92:     srcSegment = MemorySession.openImplicit().allocate(size, SPECIES.vectorByteSize());

allocateNative here

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

PR: https://git.openjdk.org/panama-foreign/pull/718