[foreign] RFR 8218669: Flatten Reference hierarchy

[Maurizio Cimadamore]

On 08/02/2019 16:41, Jorn Vernee wrote:
> Hi Maurizio,
>
> The OfDouble reference no longer checks the size of the type to 
> determine how to get/set the value. It looks like that case was 
> already handled by LayoutType.ofDouble by using References.OfLongDouble?

Yeah, not sure how that one got there. LayoutType::ofDouble creates 
always the right reference (either OfDouble or OfLongDouble).

Do you agree that the code as written now should be good?

>
> Should AbstractReference and Reference be merged as well? (and then 
> use pass up 'null' in OfGrumpy, since getter()/setter() throws any ways)

Not sure I understand what you mean - but this seems 1-1 with that we had?

I don't think we want to stash the MHs in 'instance' fields, and then 
have getter()/setter() return them - as that would be more opaque.

>
> What about profile pollution in the case of struct & array types? It 
> looks like at least OfStruct could benefit from some specialization 
> per carrier type.

I think the tension there is that if you specialize by carrier, then the 
MHs become less static (e.g. they end up depending on some instance 
field of the reference), so not-so-constant from a VM perspective (but 
perhaps @Stable could help).

With arrays we could specialize by size. But for both I'm not 100% clear 
of what the payoff would be. I'd prefer to keep these refactorings 
limited to things that have come up in JMH runs. Profile pollution with 
primitives was defo a problem (addressed by this patch); it's likely 
there will be others, but proactively fix issues w/o perf evidence to 
back things up could backfire.

Maurizio


>
> Jorn
>
> Maurizio Cimadamore schreef op 2019-02-08 13:12:
>> Hi,
>> this is a simple change which makes the Reference hierarchy in
>> References.java flatter and 1-1 with respect to the carrier it models.
>>
>> Some internal testing of linkToNative pointed at profile pollution
>> because of multiple carriers using the same reference class (e.g.
>> OfPrimitive).
>>
>> This patch does 3 things:
>>
>> 1) Remove the intermediate AbstractReference class
>> 2) Create one reference class per carrier, and make all getter/setter
>> MH static constants
>> 3) Consolidate the code my moving the Reference interface inside 
>> References.
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8218669/
>>
>> Cheers
>> Maurizio