abstract layouts

[Ty Young]

On 8/12/21 4:32 PM, Maurizio Cimadamore wrote:
>
> On 12/08/2021 19:49, Ty Young wrote:
>> My understanding is that if:
>>
>>
>> -XX:+UnlockExperimentalVMOptions
>> -XX:+TrustFinalNonStaticFields
>>
>>
>> are set then non-static final fields are optimized. Is that not true?
> Well, that's not my idea of "supported" - but ok.
>>
>>
>> Anyway, I don't want to hear anything about what I'm doing being 
>> "problematic" when the whole reason I'm doing what I'm doing is 
>> because of issues with FMA/FFI API. If you don't like it then open up 
>> layouts, otherwise I really don't want to hear it. It was well known 
>> that attributes were going to be used(and was even encouraged) for 
>> this purpose so I have no clue why all of a sudden it's a bad idea in 
>> your mind. I knew it was less optimal idea well over a year ago, but 
>> it's not like I have the power to change it.
>
> There are many things that are wrong with this statement. But let me 
> back off.
>
>
> In general, when the design of an API evolves, one learn more things. 
> The main reason why layout attributes were added in the first place 
> was (solely) to support metadata which would help the linker going 
> about its business. Then, since we were there, we decided to open up 
> the machinery.
>
> I think it is legitimate, after some time, to ask the question of 
> who's using layout attributes (apart from the linker itself) and why. 
> If it turns out that there's little uses for the attribute API, and 
> that the few uses there are can be done in other ways, then I think it 
> is a legitimate question to ask whether we want to expose layouts 
> attributes in the final API.
>
> As for "why not just use inheritance" which has been brought up other 
> times too, I gave an explanation earlier today; having immutable 
> classes with withers, and have them also extensible via subclassing is 
> not something that is very common. It's either one or the other, 
> typically. So, it's not like there's an obvious solution to this 
> problem which we are failing for some reason to see - it's either:
>
> 1. keep attributes as they are
> 2. drop attributes and allow inheritance (but with the 99% chance of 
> having broken subclasses)
> 3. drop attributes period
>
> I don't think (2) is such a great place to be. So it's either (1) or 
> (3). Your use case doesn't shift the balance one way or another, as 
> you can do same things even w/o layout attributes, sorry. But maybe 
> somebody else has a different use case, in which case I'd be 
> interested to hear.


The gaslighting is strong in this statement. You have never up to this 
point even indicated that attributes may be removed. In fact, up to this 
point, you have at the very least implied that attributes were going to 
be a thing for higher-level APIs that need decorating layouts in order 
to work, never that it was solely for linker that I can remember. You 
yourself claimed that higher level APIs would be supported, knowing that 
layouts and their attributes were used in this exact same way as it is 
here because the source code was public at the time.


And to add insult to injury you're suggesting that there is some magical 
way in which I can support all use cases by... doing what? Creating 
explicit dereference types for every standard type? How deep should I go 
with that? 1 explicit dereference class for each type? 2? 3? 4? 10? For 
someone who refuses to add 3 tiny and simple methods and a small API 
change and doesn't seem to want to breakout GroupLayout into 
StructLayout and UnionLayout, that suggestion is a bit ironic.


I've probably used Panama since the beginning, have promoted it, have 
one of the few real-world actual applications that use it, and have 
reported bugs that somehow people working on Panama full time weren't 
able to find *because* I have an actual application that uses Panama and 
not just a few classes that are modified from using Unsafe/ByteBuffers, 
which seems to be what everyone else is doing and **cannot** be compared 
to my projects. To claim that my use case is somehow not valid and 
gaslight out of such a small sample side(1/1, AFAIK) sure is a slap in 
the face. Seems to be a trend with the death of the system wide JRE(not 
really, there are JREs and programs that need them but we'll pretend 
otherwise) and the deprecation of SecurityManager,  so I guess at least 
you're being consistent.


Is there even anyone else using Project Panama in a real-world GUI 
application or something? Even internally at Oracle? If not, FFI support 
should be removed too. Since so few people are using it, It's too much 
API baggage for too little return! If a sample size of 1 outside of 
Oracle isn't enough to keep attributes then surely it's not enough to 
keep FFI support, right? I mean, JNI/JNA/GraalVM/other projects exist 
already, why complicate things further? It's such a corner-case and not 
worth supporting.


>
> (For the records, when thinking about (3) I also did an exercise and I 
> went back and reimplemented jextract - which uses 4-5 different layout 
> attributes. Turns out none of them was _really_ needed and in fact the 
> code ended up cleaner w/o them).


Great! Don't remember it ever being said it was explicitly for jextract 
until... now? Over a year(2? So long ago!) later?


>
> Maurizio
>
>
>>
>>
>>
>> On 8/12/21 1:27 PM, Maurizio Cimadamore wrote:
>>> I think what you are trying to do is problematic.
>>>
>>> You have NativeInteger, and NativeInteger has a static layout. You 
>>> want to attach other static information (method handles). The 
>>> information has to be _static_ if you want to have any chance of the 
>>> JVM to optimize all that.
>>>
>>> But, with attributes, you are storing the MHs in a map - so, not 
>>> static. Even if you could declare your own layout class, storing the 
>>> method handles in final fields of the custom layout won't help (if 
>>> the final fields aren't trusted by the JVM, which they are not, 
>>> except for special provisions for some JDK packages).
>>>
>>> In other words, I don't see a path (besides spinning a new 
>>> NativePointer64 instance which is exclusively dedicated to 
>>> NativeInteger) by which you can get performance when accessing the 
>>> NativeInteger from the NativePointer64.
>>>
>>> But, if you don't care much about inlining/performance, then maybe 
>>> you can just use a ClassValue and store all the MethodHandles 
>>> related to NativeInteger in there? That is going to provide similar 
>>> (if not better) performance than those with layout attributes.
>>>
>>> I guess what I'm trying to say is that, unless I misunderstood your 
>>> example, it seems another case of "layout attributes are attractive 
>>> nuisance" - e.g. once they are there, one is tempted to just inject 
>>> stuff into layouts, but you already have custom classes wrapping 
>>> layouts, so why don't you associate the (static) metadata with the 
>>> classes, rather than the layouts?
>>>
>>> Maurizio
>>>
>>> On 12/08/2021 19:00, Ty Young wrote:
>>>> Because it's needed for a NativePointer64 type:
>>>>
>>>>
>>>> NativePointer64<NativeInteger> intPointer = new 
>>>> NativePointer64<>(NativeInteger.LAYOUT);
>>>>
>>>>
>>>> NativeInteger has 3 MethodHandles embedded into it, one which 
>>>> accepts nothing(safe), one which accepts a MemorySegment(unsafe), 
>>>> and one which accepts a MemoryAddress(unsafe, what is used by 
>>>> NativePointer64). NativePointer then takes the given layout, embeds 
>>>> it inside of itself, then reads it later when getting the 
>>>> dereferenced value. Doing this generates garbage because of 
>>>> Optional since the NativePointer64.LAYOUT needs to have 
>>>> NativeInteger embedded inside of it, which then needs to be 
>>>> resolved according to a type attribute, e.g. enum, pointer, number. 
>>>> It's wasteful(in terms of memory) and slow(have to go through a 
>>>> tree of static methods to resolve). Layouts being opened up would 
>>>> help.
>>>>
>>>>
>>>>
>>>> On 8/12/21 8:39 AM, Maurizio Cimadamore wrote:
>>>>> May I suggest, for the benefit of the conversation, that instead 
>>>>> of focusing on the _hows_ we focus instead on the _whys_ ?
>>>>>
>>>>> E.g. "I'd like to decorate a layout with information X and Y, 
>>>>> because I use these information in order to do Z"
>>>>>
>>>>> In other words, by having a full picture of what you are trying to 
>>>>> do we might understand better what the requirements really are.
>>>>>
>>>>> Thanks
>>>>> Maurizio
>>>>>
>>>>> On 12/08/2021 14:12, Maurizio Cimadamore wrote:
>>>>>>
>>>>>> On 12/08/2021 12:24, Ty Young wrote:
>>>>>>> I'd very much like it if layouts were opened up and non-sealed. 
>>>>>>> It would allow for less wasteful(no Optional) and faster code(no 
>>>>>>> HashMap, things can be final). 
>>>>>> I agree that would work better.
>>>>>>
>>>>>>> It also would be nice if GroupLayout was removed and replaced 
>>>>>>> with StructLayout and UnionLayout, or used as a shared/common 
>>>>>>> interface/class for them.
>>>>>> That is largely orthogonal and irrelevant in the discussion we're 
>>>>>> having here.
>>>>>>>
>>>>>>>
>>>>>>> I feel like the with* issues could be fixed with generics, e.g.:
>>>>>>>
>>>>>>>
>>>>>>> public interface MemoryLayout<L extends MemoryLayout<?>> {
>>>>>>>
>>>>>>>     public L withName(String name);
>>>>>>>
>>>>>>> }
>>>>>>
>>>>>> I don't think this solves much - as you will have, likely:
>>>>>>
>>>>>> ValueLayout extends MemoryLayout<ValueLayout>
>>>>>>
>>>>>> At which point you're back to square one - if you define 
>>>>>> `TyLayout extends ValueLayout`, there's nothing forcing you to 
>>>>>> override withName.
>>>>>>
>>>>>> I did experiments adding generic types to all layouts two years 
>>>>>> ago and honestly, it doesn't work very well. About 99% of the 
>>>>>> times, the most helpful thing the client has to say about a 
>>>>>> layout is MemoryLayout<?>, which seems like forcing 99% of the 
>>>>>> users to use generics because 1% of use cases might need them. 
>>>>>> But that's beside the point - generics do not solve the problem I 
>>>>>> was talking about. Also, in my experience, having f-bounded type 
>>>>>> variables in hierarchies works very well when you have one top 
>>>>>> class and a single level of leaves (which are all peers). It 
>>>>>> works much less when you have to start defining sub-hierarchies 
>>>>>> in the leaves - because either the intermediate nodes are 
>>>>>> conscious about the type variables and leave them open (leading 
>>>>>> to very verbose and borderline uncomprehensible generic 
>>>>>> declarations), or they fix the type variables, in which case the 
>>>>>> leaves will not take advantage of the f-bound, and you'll be back 
>>>>>> in no-generic-land.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> All of the basic layout types should be interfaces, not classes 
>>>>>>> IMO. Basic implementations could be put in a non-exported 
>>>>>>> package. Users can get a basic implementation from MemoryLayout 
>>>>>>> static methods, as they do now.
>>>>>> Again, this seems a bit of a shallow comment. We can turn layouts 
>>>>>> from interface to classes in 2 minutes, by doing a targeted 
>>>>>> refactoring. But again, whether classes or interfaces, the 
>>>>>> problem I pointed out is still there and is the one we'd need to 
>>>>>> solve to be able to let clients to subclass layouts freely.
>>>>>>>
>>>>>>>
>>>>>>> I don't think what I'm proposing could be done with static 
>>>>>>> decorator methods. The information being added as attributes 
>>>>>>> include MethodHandles for the class's constructor, e.g. a 
>>>>>>> constructor MethodHandle for a NativeInteger class, something 
>>>>>>> that specific can only be done at the class level I think.
>>>>>>
>>>>>> Well, the original proposal was to add a bunch of methods:
>>>>>>
>>>>>> * MemoryLayout::isAbstract
>>>>>> * MemroyLayout::asAbstract
>>>>>> * MemoryLayout::unfilledAttributes
>>>>>>
>>>>>> If we have layout attributes (like we do in today's API), there 
>>>>>> is no need for these methods to _be_ in MemoryLayout. They can be 
>>>>>> static methods taking a layout, defined wherever you want. Just 
>>>>>> use a special name to encode the attribute names - e.g. 
>>>>>> "abstract/foo" - then:
>>>>>>
>>>>>> * isAbstract --> does the layout have any attribute whose name 
>>>>>> starts with "abstract/" which are set to null?
>>>>>> * asAbstract --> encode the attribute names to use the 
>>>>>> "abstract/" prefix
>>>>>> * unfilledAttributes -> get all the attributes whose name start 
>>>>>> with "abstract/" and return those that have no value set
>>>>>>
>>>>>> Maurizio
>>>>>>
>>>>>>
>>>>>>
>>>>>>