[foreign-jextract] RFR: 8264187: Add a method for creating a slicing MethodHandle

[Maurizio Cimadamore]

On Fri, 26 Mar 2021 12:20:02 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> Hi,
> 
> This patch adds a `sliceHandle(PathElement...)` method to MemoryLayout, which can be used to create a method handle that, when given a segment, will return a slice of that segment for the layout element selected by the earlier given layout path.
> 
> This for instance makes it easier to iterate over an array of structs by first creating a slice for each element, and then accessing struct fields from there, without having to resolve to manual offset computation. For example:
> 
> SequenceLayout seq = MemoryLayout.ofSequence(5,
>     MemoryLayout.ofStruct(
>        MemoryLayout.ofValueBits(32, ByteOrder.nativeOrder()).withName("x"),
>        MemoryLayout.ofValueBits(32, ByteOrder.nativeOrder()).withName("y")
>     ));
> 
> MethodHandle sliceHandle = seq.sliceHandle(sequenceElement());
> 
> MemoryLayout structLayout = seq.select(sequenceElement());
> VarHandle xHandle = structLayout.varHandle(int.class, groupElement("x"));
> VarHandle yHandle = structLayout.varHandle(int.class, groupElement("y"));
> 
> try (ResourceScope scope = ResourceScope.ofConfined()) {
>     MemorySegment segment = MemorySegment.allocateNative(seq, scope);
>     long count = seq.elementCount().orElseThrow();
>     for (int i = 0; i < count; i++) {
>         // create view segment of single struct element
>         MemorySegment struct = (MemorySegment) sliceHandle.invokeExact(segment, (long) i);
>         xHandle.set(struct, 42);
>         yHandle.set(struct, 84);
>     }
> }
> 
> Thanks,
> Jorn

Looks good - I've left some comments

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryLayout.java line 497:

> 495: 
> 496:     /**
> 497:      * Creates a method handle which can be used to create a slice of the layout selected by a given layout path,

I'd say "which, given a memory segment, returns a slice (hyperlink to MemorySegment::asSlice) corresponding to the layout selected by a given layout path"

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryLayout.java line 516:

> 514:      * where {@code x_1}, {@code x_2}, ... {@code x_n} are <em>dynamic</em> values provided as {@code long}
> 515:      * arguments, whereas {@code c_1}, {@code c_2}, ... {@code c_m} and {@code s_0}, {@code s_1}, ... {@code s_n} are
> 516:      * <em>static</em> stride constants which are derived from the layout path.

There is an issue here, which is probably not introduced by you, but also present in other methods: c_1 and s_0 are not all strides; c_0 are offsets, while s_0 are strides.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryLayout.java line 523:

> 521:      * }</pre></blockquote>
> 522:      *
> 523:      * where {@code segment} is the segment to be sliced, and where {@code layout} is the layout selected by the given

Maybe we can say "as per MemoryLayout::select(path)" ?

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/LayoutPath.java line 76:

> 74:             MH_ADD_SCALED_OFFSET = lookup.findStatic(LayoutPath.class, "addScaledOffset",
> 75:                     MethodType.methodType(long.class, long.class, long.class, long.class));
> 76:             MH_SLICE = lookup.findStatic(LayoutPath.class, "slice",

Why not just pointing to MS::asSlice?

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/LayoutPath.java line 211:

> 209: 
> 210:     public MethodHandle sliceHandle() {
> 211:         MethodHandle offsetHandle = offsetHandle(); // bit offset

Nice composition of existing bits!

test/jdk/java/foreign/TestLayoutPaths.java line 533:

> 531:         MethodHandle sliceHandle;
> 532:         try {
> 533:             sliceHandle = layout.sliceHandle(groupElement("y")); // should work

Question: why is the failure dynamic? We know exactly, at construction time that the offset is gonna be odd?

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

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