[foreign-memaccess] RFR: 8248487: Add static helpers to access segments

[Jorn Vernee]

On Mon, 29 Jun 2020 16:40:19 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This patch addresses some usability issues in the foreign memory API. First, it adds several static accessors for all
> carrier types in MemoryLayouts (I'm thorn whether to add this in MemoryLayouts or MemoryHandles, suggestions welcome).
> These accessors provide an access API that is similar to unsafe - all accessors expect a base address plus a byte
> offset. In most cases the accessors can be used and direct VH usage can be dropped, which might make approaching this
> API a bit easier.  Secondly, I've added a bunch of helpers to map a segment into _any_ array (as opposed to just
> `byte[]`). This makes the API more symmetric, given that, after all we _were_ able to create a segment from _any_ array
> (not just `byte[]`).

Marked as reviewed by jvernee (Committer).

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryLayouts.java line 116:

> 115:      * @param offset offset (relative to {@code addr}). The final address of this read operation can be expressed
> as {@code addr.addOffset(offset)}. 116:      * @return a char value read from {@code addr} at the offset specified by
> {@code index}. 117:      */

Here, and similarly elsewhere:
Suggestion:

     * @return a char value read from {@code addr} at the offset specified by {@code offset}.

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

> 409:      * @throws UnsupportedOperationException if this segment does not feature the {@link #READ} access mode, or if
> this 410:      * segment's contents cannot be copied into a {@link short[]} instance, e.g. because {@code byteSize() %
> 4 != 0}, 411:      * or {@code byteSize() / 2 > Integer#MAX_VALUE}.

Typo
Suggestion:

     * segment's contents cannot be copied into a {@link short[]} instance, e.g. because {@code byteSize() % 2 != 0},

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

> 629:         Consumer<MemoryAddress> doubleInitializer =
> 630:                 (base) -> initBytes(base, doubles, (addr, pos) -> MemoryLayouts.setDouble_BE(addr, pos * 8,
> (double)(long)pos)); 631:

Seems strange to me to switch to MemoryLayout.setXXX here, since we are actually accessing an element of a known
layout, instead of a one-off value.

As a result, you now have to multiply the offset with a magic number that represents the stride. I don't think this is
a good use-case...

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

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