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

[Erik Österlund]

Hi Roman,

On 2018-03-14 14:19, Roman Kennke wrote:
> 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.

Yes, this was precisely why I wanted this.

> 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?

Admittedly, that does make me a little bit sad. And I still wonder if 
LTO techniques could save the day. But if it can't then I don't have any 
other better ideas than to explicitly call some equals function 
everywhere we compare oops.

Thanks,
/Erik

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