[foreign-memaccess] RFR 8226421: Remove LayoutPath API

[Maurizio Cimadamore]

Hi

On 25/06/2019 13:25, Jorn Vernee wrote:
> Hi,
>
> There are good things in this patch; 1.) I like the fact that group 
> elements can no longer be accessed by index, as well as 2.) the fact 
> that we can now only use compound layouts or value layouts as a root 
> path element.
>
> But, these seem orthogonal to the switch to using path expressions, 
> and I'm a little more skeptic about that. With path expressions we have;
>
> * one less interface in the API (but complexity still exists in grammar)
> * more compact to create
>
> But imho one of the biggest problems with LayoutPaths was that it was 
> still possible to create a path that pointed to an element that could 
> not be de-referenced, which is still possible with path expressions. 
> Moreover, this removes the static validity of a path, because it's 
> possible to mess up the expression string. I think LayoutPath has the 
> following pros compared to path expressions;
I'm honestly not super convinced this is a big problem... when you 
create a path, the path must end in a leave (e.g. a value layout). This 
is actually easier to enforce in the new expression-based API than it 
was in the path-based world (since a path was just ... a path).
>
> * API speaks for itself more (no need for separate grammar description 
> to describe what's possible)
I can buy that
> * Static validity of the path

Less certain about this - if you mean that, once you construct a chain 
of layout path calls you are certain that the path is good, I think this 
might be too optimistic - you could still have selected something that 
doesn't make sense (e.g. select field "foo" that's not declared in the 
struct). So - not sure what yo mean by 'static' validity - both variants 
need to be checked against the layout - and errors reported accordingly.

The only advantage I see is that it's not possible to write an 
ill-formed path (e.g. `foo[bar]`) with the API-based approach because 
the API is itself the grammar. Of course you trade this advantage off 
with compactness.

>
> And, personally, I like these pros more than the ones we get from path 
> expressions.
>
> Maybe I'm missing some pros/cons here (as I was not part of the 
> earlier talks). But, currently, I think I'd prefer sticking with 
> LayoutPath, and implementing just 1. and 2. There are some other ideas 
> like spelling out the valid carriers with different xxxHandle methods 
> (charHandle, byteHandle, etc.) to at least partially close the invalid 
> carrier type hole. Or replace the toPath-like methods in Layout with 
> static factory methods in LayoutPath (like we had before), to decouple 
> Layouts from LayoutPath a bit more.

The main issue with the layout path API is that is fairly front and 
center in the current memory access API, whereas in reality its role is 
fairly minor (e.g. select a leaf layout from a compound, so that we can 
compute some var handle, or some offset). Although I can agree that the 
layout path API route is self-consistent (which is why we went there), 
the general feeling is that a toplevel abstraction for this seems a tad 
on the heavy side.

One alternative would be to turn LayoutPath into some kind of functional 
interface:

VarHandle handle = groupLayout.dereferenceHandle(path -> path
                                   .groupElement("foo")
.sequenceElement(42)
                                    .groupElement("bar"));

While this is similar in spirit to what we currently have, I can see 
some benefits:

* we can still add the very (very!) handy ValueLayout::dereferenceHandle 
(which basically allows to avoid uses of paths in majority of cases)

* Usefulness of the LayoutPath API is more clearly defined inside the 
boundaries of the lambda bubble

* Which means we can define LayoutPath as a static interface inside 
Layout - e.g. Layout.Path - which is less likely to attract attention


Thoughts?

Maurizio


>
> Jorn
>
> On 2019-06-20 00:34, Maurizio Cimadamore wrote:
>> Hi,
>> now that we have combinators to create memory access handles, it seems
>> like there is less need to have a very rich API to describe layout
>> paths. But, we still want some higher level way to create var handles
>> (or compute offsets) from a complex layout expression...
>>
>> Chatting with Brian and John, an idea emerged - to use a simple
>> grammar to describe a layout path.
>>
>> For instance, if you have the usual array of struct we used in other 
>> examples:
>>
>> [ 5 : [ x32 i32(value) ] ]
>>
>> A path from the outermost sequence to the 'value' layout element can
>> be described with the following string:
>>
>> "[].value"
>>
>> So, if we had a language to describe layout paths, we can replace the
>> LayoutPath API with some methods which take a path expression as
>> argument.
>>
>> This is the patch:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8226421/
>>
>> I think the results are quite pleasing, and more compact than before.
>> I also placed the VarHandle accessors where they belong - that is, an
>> accessor taking a path expression in CompoundLayout. And then one that
>> just takes a carrier in ValueLayout (as no selection is possible from
>> a leaf).
>>
>> I think this takes care of the last 'shaky' abstraction in the memory
>> access API.
>>
>> Impl-wise, the expression parser builds on some of the work we've done
>> in the foreign branch - I've added a test which checks several 'bad'
>> things that can happen during parsing.
>>
>> I've also added a javadoc section on 'path expressions' - which shows
>> the full grammar production for the path expression language.
>>
>> One thing that I removed in the process was the ability to refer to
>> struct components by index - in part because it felt a bit unjustified
>> (for now, at least), in part because it also felt a bit shaky (w.r.t.
>> unions).
>>
>> Cheers
>> Maurizio