[foreign-memaccess+abi] RFR: 8310362: Improve composability of handle derived from layouts [v2]

[Maurizio Cimadamore]

On Thu, 22 Jun 2023 19:21:35 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Me and Maurizio have been discussing how to better address certain use cases like accessing structs with variable length array members, or matrices that have a dynamic row and column size.
>> 
>> The solution we came up with is captured in this patch. It adds an additional base offset parameter to the handles produced by `MemoryLayout::varHandle`, `MemoryLayout::sliceHandle` and `MemoryLayout::byteOffsetHandle`. This allows these handles to be composed with other offset computations that are not necessarily derived from layouts.
>> 
>> This patch also adds a `scale` method to MemoryLayout, which can be used to scale and index by the size of a layout. Essentially: `offset + layout.byteSize() * index`. This is useful for expressing ad-hoc offset computations. A `scaleHandle` method is also added for convience, which returns a MethodHandle for `scale` bound to the receiver layout.
>> 
>> Both these changes allowed for several simplifications:
>> - `MethodHandles::memorySegmentViewVarHandle` can be removed, as it's now exactly the same as calling ML::varHandle on a ValueLayout with an empty layout path. (the former method also didn't take advantage of the access handle cache found on ValueLayouts).
>> - `ValueLayout::arrayHandle` is removed, as most use cases can now simply be expressed using ML::varHandle + ML::scaleHandle. e.g.:
>> 
>> MethodHandles.collectCoordinates(layout.varHandle(),
>>             1, MethodHandles.insertArguments(layout.scaleHandle(), 0, 0L));
>
> Jorn Vernee has updated the pull request incrementally with five additional commits since the last revision:
> 
>  - missing changes
>  - rework offset computation doc
>  - address most review comments
>  - add collectCoordinates void filter review changes
>  - include root layout check review changes

src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 343:

> 341:      */
> 342:     @ForceInline
> 343:     default long scale(long offset, long index) {

was thinking... do we want the output of this method to ever be negative? (since it's used as an offset to a segment). Also, should we check for overflows with multiplyExact?

src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 387:

> 385:      * <ul>
> 386:      *     <li>its return type is {@code long};</li>
> 387:      *     <li>it has at least one {@code long} leading parameter representing the base offset;</li>

should be "it has one" e.g. not "at least one" right?

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

PR Review Comment: https://git.openjdk.org/panama-foreign/pull/840#discussion_r1239027905
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/840#discussion_r1239028333