RFR: 8231497: [lworld] Inline type use of Access API (Runtime)

[Frederic Parain]

Sounds good.

Reviewed with the name change below.

Thank you,

Fred


> On Sep 27, 2019, at 07:08, David Simms <david.simms at oracle.com> wrote:
> 
> 
> I agree with your concern here; propose "value_alloc_copy_from_index()" for the return oop method ?
> 
> 
> On 26/09/19 5:46 PM, Frederic Parain wrote:
>> In addition of fixing some GCs issues, it also makes the code
>> much cleaner!
>> 
>> Thank you for having included the handling of empty values in
>> the copy methods, it will be easier to maintain.
>> 
>> I have an issue with method naming in valueArrayOop.hpp:
>> 
>>  41   static oop value_copy_from_index(valueArrayHandle vah, int index, TRAPS);
>>  42   void value_copy_from_index(int index, oop dst) const;
>> 
>> Two methods with the same name, but one returns an oops, the other void,
>> one is static, one is not. Even if arguments are different, I’m concerned
>> that this overloaded name could lead to confusion.
>> 
>> Regards,
>> 
>> Fred
>> 
>> 
>> 
>>> On Sep 26, 2019, at 10:23, David Simms <david.simms at oracle.com> wrote:
>>> 
>>> 
>>> 
>>> Bug/RFE: https://bugs.openjdk.java.net/browse/JDK-8231497
>>> 
>>> Webrev: http://cr.openjdk.java.net/~dsimms/valhalla/8231497/webrev0/
>>> 
>>> 
>>> 
>>> Hi,
>>> 
>>> So for the longest time Valhalla has completely ignored the Heap Access API, and gone and used write barriers directly, thereby breaking other GCs with different barrier requirements.
>>> 
>>> Here is a 1st pass at adding the required Heap Access API for "inline types", and an runtime (C++) implementation for modBarrierSet and zBarrierSet. No compiler support as yet, I've open another bug[1] for that.
>>> 
>>> Webrev notes:
>>> 
>>> 	• Reviewed the actual Access API changes already with Erik Österlund, he's okayed it.
>>> 	• There was some inline header file clean up required, sorry for the noise
>>> 	• Hotspot convention uses the Access API in *Oop and * Klass implementation, not directly in runtime code
>>> 		• ValueKlass has "value_copy_*" API since it deals with naked payload (inlined layout from different containers), not just oops (otherwise they could have gone into oop API)
>>> 	• Rather than a single "value_store", gone with multiple methods so call-site can use the appropriate decorators
>>> 		• Remove runtime conditions
>>> 		• Feel free to have an opinion on the "value_copy*" API naming
>>> 	• Passes testing
>>> 		• ValueOops -Xint, C1 and C2 need further fix [1]
>>> 
>>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8231498
>>> 
>