RFR (S): 8198561: Make oop and narrowOop always have their own class type
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
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
> 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.01_02/
> 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*
> 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.
> 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...
>>> 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
>>>> On 2/26/18 8:32 AM, Erik Österlund wrote:
>>>>> 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
>>>>> 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.
More information about the hotspot-dev