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

Frederic Parain frederic.parain at oracle.com
Thu Sep 26 15:46:19 UTC 2019


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
> 



More information about the valhalla-dev mailing list