[foreign-memaccess] RFR 8223768: Rethink relationship between Sequence vs. Group layouts

[Jorn Vernee]

> But I also think that dereferencing sub-value
> content should not be supported and best left to clients (as that
> almost always involve some bitmasking and a different set of carriers
> that is hard to be expressed in a single the API).

I think supporting this is good, as long as the sub-value matches a Java 
carrier. e.g. this would make it possible to split larger than 64 bit 
values into separate parts: u128=[2:u64] and still make them accessible 
that way, while keeping the semantic meaning that it's actually a 128 
bit value. For those cases no bitmasking is needed. Clients could create 
their own high-level carrier which wraps a `long` carrier VarHandle, and 
does multiple gets/sets on the underlying VarHandle per high-level 
get/set.

> Regardless of the decision, I think the top type 'Compound' is an
> useful abstraction.

+1

Jorn

Maurizio Cimadamore schreef op 2019-05-13 15:52:
> On 13/05/2019 14:40, Jorn Vernee wrote:
>> Hi,
>> 
>> One comment:
>> 
>> - LayoutPathImpl::sequenceElement
>> 
>>   Currently, looking up sequence elements as contents of a Value does 
>> not work. e.g.
>> 
>>   Value v = Value.ofUnsignedInt(64).withContents(Sequence.of(8, 
>> Value.ofUnsignedInt(8)));
>>   VarHandle vh = 
>> v.toPath().sequenceElement().dereferenceHandle(byte.class); // Boom! 
>> IllegalStateException in LayoutPathImpl::sequenceElement
>> 
>>   If this case is not meant to be supported, I think the type of 
>> Value::contents should just be Optional<Group> instead of 
>> Optional<Sequence>.
> 
> There are two parts to this: I think constructing paths to sub-value
> contents should work. But I also think that dereferencing sub-value
> content should not be supported and best left to clients (as that
> almost always involve some bitmasking and a different set of carriers
> that is hard to be expressed in a single the API).
> 
> So, the idea is that deferencing a path that points inside the guts of
> some value should throw some sane exception (I have some code which
> makes this more explicit).
> 
> Of course, when we get to a more general version of
> compoundElement(long) which works on both sequences and groups, we
> should make sure that, given your layout, this works:
> 
> v.toPath().compoundElement(2);
> 
> A separate question is: given we don't intend to support dereference
> of value innards, should we keep the Value::contents API?
> 
> Regardless of the decision, I think the top type 'Compound' is an
> useful abstraction.
> 
> Maurizio
> 
>> 
>> Cheers,
>> Jorn
>> 
>> Maurizio Cimadamore schreef op 2019-05-13 14:14:
>>> Hi,
>>> as discussed previously, the relationship between sequences and 
>>> groups
>>> is a bit shaky, as there are sequences that cannot be expressed as
>>> groups (sequences whose size is unbound).
>>> 
>>> This patch models groups and sequences independently:
>>> 
>>> * they all have a (minimal) common supertype called Compound (which
>>> supports only a Stream<Layout> elements() method)
>>> 
>>> * Group implements Iterable, always has a size, and you can access
>>> elements by index
>>> 
>>> * Sequence doesn't implement Iterable, has an optional size, and has
>>> an accessor to retrieve the element layout (there's only one of them,
>>> regardless of the size)
>>> 
>>> I think this is much saner.
>>> 
>>> Webrev:
>>> 
>>> http://cr.openjdk.java.net/~mcimadamore/panama/8223768/
>>> 
>>> P.S.
>>> of course Compound should be renamed to CompoundLayout in a later
>>> webrev (as for all other layout names).
>>> 
>>> P.P.S.
>>> in a separate patch I'll explore generalizing this method:
>>> 
>>> LayoutPath::groupElement(long index)
>>> 
>>> with this:
>>> 
>>> LayoutPath::compoundElement(long index)
>>> 
>>> Since now we should be able to implement indexed access on both group
>>> and sequences.
>>> 
>>> Maurizio