RFR (S): 8198561: Make oop and narrowOop always have their own class type (original) (raw)

Roman Kennke rkennke at redhat.com
Wed Mar 14 13:19:22 UTC 2018

Alright, thank you for the explanations.

The main reason why I wanted this change was so that we could overload == (i.e. equality comparison of oops), and redirect it through BarrierSet or Access API.

Since this is not possible on pointers, e.g. oopDesc*, which is what oop is typedef'd to in release builds, the next reasonable option is to provide an explicit static method in oopDesc, e.g. oopDesc::equals(oop, oop) (and narrowOop version) which would then call into BarrierSet or Access APIs.

This would not be unprecedented: we already have oopDesc::is_null(oop) and oopDesc::compare(oop, oop).

In Shenandoah land, we already know all the places where to put oopDesc::equals() instead of ==, and we do have some code in oopsHierarchy to overload == in fastdebug builds and verify to not call naked == on oops.

Would that be a reasonable way forward? If yes, then I can provide an RFR soon.

WDYT?

Roman

1. I looked at the generated machine code from a bunch of different compilers and found that it was horrible. 2. Found that the biggest reason it was horrible was due to unfortunate uses of volatile in oop. Was easy enough to solve:

Full webrev: http://cr.openjdk.java.net/~eosterlund/8198561/webrev.02/ Incremental: http://cr.openjdk.java.net/~eosterlund/8198561/webrev.0102/ 3. Found that even after solving the accidental volatile problems, oops sent as value as arguments to functions were sent on the stack (not register), because oop is not a POD, and this is seemingly a strict ABI requirement of C++. Optimizing it would be an ABI violation and is therefore not done. 4. Found that oop is inherently not going to be a POD unless we rewrite a lot of code in hotspot due to having e.g. volatile copy constructor (which can not be auto generated) and a popular user defined oopDesc* constructor. 5. Got sad that C++ really has to send wrapper objects as simple as oop through the stack on callsites and dropped this as I did not know how I felt about it any longer. You can pick this up where I left if you want to and check if the performance impact due to the suboptimal machine code is something we should be scared of or not. If there is reason to be scared, I wonder if LTO mechanisms can solve part of this and a whole bunch of unnecessary use of .inline.hpp files at the same time. Thanks, /Erik On 2018-03-14 12:27, Roman Kennke wrote: So where are we with this change?

There's not many places where I can think of possible performance problems. Probably the most crucial ones are the oop/narrowOop/object iterators that are used by GC. OopClosure and subclasses get pointers to oop/narrowOop.. it shouldn't make a difference. Then there's ObjectClosure which receives an oop. Does it make a difference there? Maybe write a little benchmark that fills the heap with many small objects, and runs an empty ObjectClosure over it? If it doesn't show up there, I'm almost sure it's not going to show up anywhere else... Roman

Hi Coleen,

Thanks for the review. On 2018-02-26 20:55, coleen.phillimore at oracle.com wrote: Hi Erik,

This looks great.   I assume that the generated code (for these classes vs. oopDesc* and juint) comes out the same? I assume so too. Or at least that the performance does not regress. Maybe I run some benchmarks to be sure since the question has been asked. Thanks, /Erik thanks, Coleen On 2/26/18 8:32 AM, Erik Österlund wrote: Hi,

Making oop sometimes map to class types and sometimes to primitives comes with some unfortunate problems. Advantages of making them always have their own type include: 1) Not getting compilation errors in configuration X but not Y 2) Making it easier to adopt existing code to use Shenandoah equals barriers 3) Recognize oops and narrowOops safely in template Therefore, I would like to make both oop and narrowOop always map to a class type consistently. Webrev: http://cr.openjdk.java.net/~eosterlund/8198561/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8198561 Thanks, /Erik

More information about the hotspot-dev mailing list