[foreign-memaccess] RFR 8223712: Investigate more fluent LayoutPath API

[Maurizio Cimadamore]

On 10/05/2019 18:27, Jorn Vernee wrote:
> Hi,
>
> I think this is a nice move!
>
> One minor nit: In LayoutPathImpl::sequenceElement there is an 
> unspecified throw of IllegalStateException. Should probably be a UOE 
> instead?
Yep - sorry, leftover.
>
> There's also still a bit of a split between Sequence and Group. For 
> instance, it is not possible to create a LayoutPath to an exact 
> element of a sequence, and there is no way to make a LayoutPath to a 
> group, where you would pass in the name/index of the element you want 
> to access to the VarHandle later. Maybe it could be nice to add 
> support for these missing cases?
I think the former idea is probably interesting to explore (access array 
at position i, fixed). Other stuff, not so much - in other words, 
VarHandle access needs to be fast - so the coordinates have to be 
something which can be translated into an offset in O(1) [w/o 
allocating]. Looking up things in a map dynamically doesn't fall in this 
category.
>
> Like you, I'm also a bit skeptical about the inheritance relationship 
> between Sequence and Group. e.g. if you look at 
> LayoutPathImpl::lookupGroup, any Group is a valid argument, except if 
> it's a Sequence. This seems to violate LSP. I think it might be better 
> to have Sequence inherit from AbstractLayout directly instead?

Right, that's what got me started on this topic... (although I've been 
aware of the issues esp. w.r.t. unspecified sequences for a long time).

Maurizio

>
> Cheers,
> Jorn
>
> Maurizio Cimadamore schreef op 2019-05-10 17:24:
>> Hi,
>> after some discussion with Brian, here's an attempt to make the
>> LayoutPath lookup API simpler, by removing the Stream-returning lookup
>> method.
>>
>> Now, you just ask a path out of a layout (the path you obtain is
>> rooted there), and then you proceed from there calling various
>> builder-like methods - example:
>>
>> Sequence seq = Sequence.of(20,
>>                 Sequence.of(10,
>>                       Group.struct(
>>                             Padding.of(32),
>>                             Value.ofUnsignedInt(32).withName("elem")
>>                       )
>>                 ));
>>
>>
>> seq.toPath()
>>                 .sequenceElement()
>>                 .sequenceElement()
>>                 .groupElement("elem")
>>
>>
>> I've also added support for 'by-index' lookups, so the above can also
>> be written by this:
>>
>> seq.toPath()
>>                 .sequenceElement()
>>                 .sequenceElement()
>>                 .groupElement(1)
>>
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8223712/
>>
>>
>> As a side-bar, I'm growing more and more skeptical about the
>> relationship between Sequence and Group; there are sequences that
>> cannot be represented as groups:
>>
>> * if sequence has more than 2^32 elements (in which case we cannot
>> create a big enough group for it)
>> * if sequence has an unknown arity (which we know in the case of
>> native interop can happen)
>>
>> The latter case is important - when you have a pointer, languages are
>> typically very loose as to whether you point to ONE element of that
>> kind, or MANY. It would be nice to be able to model these different
>> cases with different layouts.
>>
>> Maurizio