[foreign-memaccess+abi] RFR: 8273905: Foreign API refresh

[Paul Sandoz]

On Thu, 16 Sep 2021 16:39:11 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> As outlined in [1], there are areas of the foreign API we'd like to improve upon, such as:
> 
> * dereference: there is a mismatch between API points which create segments (which take a layout) and dereference API points, which do not take layouts
> 
> * role of `MemoryAddress`: in Java 17, `MemoryAddress` has become a stateful carrier, which is attached to a scope. This is inconvenient, as in most cases, a memory address is just a raw pointer that arises when interacting with native code.
> 
> * resource scopes: the API for scopes has too many flavors to pick from, many of which overlap. The fact that scopes and segment allocators are unrelated forces clients to introduce ad hoc conversions from scopes to allocators, and library developers to add overloads. Finally, the API for acquiring scopes doesn't work well with try-with-resources, and could also be simplified. 
> 
> I will add separate comments to explain how the API has changed to resolve the above issues.
> 
> [1] - https://mail.openjdk.java.net/pipermail/panama-dev/2021-September/014946.html

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/AbstractLayout.java line 238:

> 236:      * Attribute name used to specify the <em>name</em> property of a memory layout (see {@link #name()} and {@link #withName(String)}).
> 237:      */
> 238:     String LAYOUT_NAME = "layout/name";

Unused, except referenced in JavaDoc of `MemoryLayout`

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

> 44:  * follow the JVM's target platform C ABI. A C linker provides two main capabilities: first, it allows Java code
> 45:  * to <em>link</em> foreign functions into a so called <em>downcall method handle</em>; secondly, it allows to
> 46:  * generate an <em>upcalls stub</em>  which can be used to call Java method handle from a native function.

Suggestion:

 * to <em>link</em> foreign functions into a so called <em>downcall method handle</em>; secondly, it allows
 * native code to call Java method handles via the generation of <em>upcall stubs</em>.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/FunctionDescriptor.java line 212:

> 210:     }
> 211: 
> 212:     final static class VariadicFunction extends FunctionDescriptor {

Suggestion:

    static final class VariadicFunction extends FunctionDescriptor {

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 98:

> 96:  * <p>
> 97:  * Contents of mapped memory segments can be {@linkplain #force() persisted} and {@linkplain #load() loaded} to and from the underlying file;
> 98:  * these capabilities are suitable replacements for some functionalities in the {@link java.nio.MappedByteBuffer} class.

Suggestion:

 * these capabilities are suitable replacements for some capabilities in the {@link java.nio.MappedByteBuffer} class.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 114:

> 112:  * a native segment <em>unsafely</em> from a give memory address, by providing the segment size, as well as the segment {@linkplain ResourceScope scope}.
> 113:  * This is a <a href="package-summary.html#restricted"><em>restricted</em></a> operation and should be used with
> 114:  * caution: for instance, an incorrect segment size correctly could result in a VM crash when attempting to dereference

Suggestion:

 * caution: for instance, an incorrect segment size could result in a VM crash when attempting to dereference

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 135:

> 133:  * }</pre></blockquote>
> 134:  *
> 135:  * For more complex dereference operation (e.g. structured memory access), clients can obtain a <em>memory access var handle</em>,

Suggestion:

 * For more complex dereference operations (e.g. structured memory access), clients can obtain a <em>memory access var handle</em>,

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 387:

> 385:      * the length of the common prefix, and it follows that there is a mismatch
> 386:      * between the two segments at that offset within the respective segments.
> 387:      * If one segment is a proper prefix of the other than the returned offset is

Suggestion:

     * If one segment is a proper prefix of the other then the returned offset is

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 612:

> 610:      * {@code ALL-UNNAMED} in case {@code M} is an unnamed module.
> 611:      */
> 612: 

Suggestion:

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 626:

> 624:      * @throws IllegalArgumentException if the size of the native string is greater than the largest string supported by the platform.
> 625:      * @throws IllegalStateException if the size of the native string is greater than the size of the segment
> 626:      * associated with {@code addr}, or if {@code addr} is associated with a segment that is <em>not alive</em>.

Referral to unknown parameter `addr`

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java line 66:

> 64:  * Resource scopes are also closed automatically once the scope instance becomes
> 65:  * <a href="../../../java/lang/ref/package.html#reachability">unreachable</a>. This can be useful to allow for predictable,
> 66:  * deterministic resource deallocation, while still prevent accidental native memory leaks. In case a managed resource scope

Suggestion:

 * deterministic resource deallocation, while still preventing accidental native memory leaks. In case a managed resource scope

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/package-info.java line 86:

> 84:  * <em>try-with-resources</em> construct: this idiom ensures that all the memory resources associated with the segment will be released
> 85:  * at the end of the block, according to the semantics described in Section {@jls 14.20.3} of <cite>The Java Language Specification</cite>.
> 86:  * Since a resource scope act as a {@link jdk.incubator.foreign.SegmentAllocator segment allocators}, we can also

Suggestion:

 * Since a resource scope acts as a {@link jdk.incubator.foreign.SegmentAllocator segment allocator}, we can also

test/jdk/java/foreign/TestArrays.java line 62:

> 60: 
> 61:     static SequenceLayout chars = MemoryLayout.sequenceLayout(100,
> 62:             ValueLayout.JAVA_CHAR

Suggestion:

            JAVA_CHAR

And for other usages.

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

> 126: 
> 127:     static SequenceLayout ints = MemoryLayout.sequenceLayout(100,
> 128:             ValueLayout.JAVA_INT.withOrder(ByteOrder.BIG_ENDIAN)

Suggestion:

            JAVA_INT.withOrder(ByteOrder.BIG_ENDIAN)

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

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