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

[Roman Kennke]

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.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*
> 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
>>
>