[foreign-memaccess] RFR: 8249879: Split MemorySegment and MemoryAddress [v2]

[Paul Sandoz]

On Thu, 23 Jul 2020 18:17:47 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This patch splits, as the title suggests, the memory segment abstraction from the memory address abstraction, as
>> described in this email:
>> https://mail.openjdk.java.net/pipermail/panama-dev/2020-July/009928.html
>> 
>> Aside from the obvious updates to the two interfaces (and their implementation), this patch greatly simplifies the way
>> memory access var handles are generated; instead of spinning them on the fly (see `MemoryAccessVarHandleGenerator`,
>> which is now removed) now all var handles can be derived from a primitive from which takes a segment and a byte offset.
>> This caused the removal of several methods in the `JavaLangInvokeAccess` interface.  The new memory segment interface
>> has some more methods to perform slicing - which can basically be used to give a new base to an existing segment and
>> can therefore be used in a lot of cases as a replacement for `MemoryAddress::addOffset`.  The memory address interface
>> is simplified, and features an additional `segmentOffset` method, which returns the byte offset of an address relative
>> to the given segment (this is similar to the old `MemorySegment::rebase` method, which is now removed).  The
>> `MemoryAccess` class needed a lot of tweaks in the various signatures, to take a segment instead of a base address.  I
>> tried to update the documentation as best as I could, but it's possible I missed some references to the old
>> relationship between segment and addresses, please double check.  Of course integrating this patch is gonna cause
>> issues in foreign-abi and foreign-jextract, as these branches make (heavy) use of memory access var handles. I suspect
>> that integration will trigger a merge failure, so we can fix followup issues as part of the merge process, so that we
>> can guarantee that all the branches remain buildable.
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review comments from Henry

src/java.base/share/classes/java/lang/invoke/X-VarHandleMemoryAccess.java.template line 99:

> 98:         long address = base + offset;
> 99:         if (skipOffsetCheck) {
> 100:             //note: the offset portion has already been aligned-checked, by construction

There are two alignment checks, one to the type size in bytes itself, and another to presumably multiples of the type
size, otherwise the first would fail. The name of `skipOffsetCheck` is a little confusing, `skipAlignmentMaskCheck`
would be more verbose but accurate. Documenting the rational on `MemoryAccessVarHandleBase` would i think help
maintainers/reviewers of the code.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryHandles.java line 49:

> 48:  * to emulate different addressing modes. The var handles created by this class feature a <em>mandatory</em>
> coordinate type 49:  * (of type {@link MemorySegment}), and one {@code long} coordinate types, which represents the
> offset, in bytes, relative 50:  * to the segment, at which dereference should occur.

s/type/types

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java line 106:

> 105:      * The <em>unchecked</em> memory address instance modelling the {@code NULL} address. This address is
> <em>not</em> backed by 106:      * a memory segment and hence it cannot be dereferenced.
> 107:      */

Refers to "backed by a memory segment".

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java line 72:

> 71:      */
> 72:     long segmentOffset(MemorySegment segment);
> 73:

It might be worth explaining how this works e.g. equivalent to `this.toRawLongValue() -
segment.address().toRawLongValue()`, and pointing out the resulting offset can be beyond the spatial bounds of the
segment (negative value, or greater).

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

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