RFR: JDK-8199735: Mark word updates need to use Access API

[Erik Österlund]

Hi Roman,

This looks good to me. Thank you for doing this.
The only thing I am a bit puzzled by is why the oopDesc::set_mark member 
function takes a volatile markOop as parameter. That seems rather weird. 
But that is unrelated to your changes.

Thanks,
/Erik

On 2018-03-25 23:03, Roman Kennke wrote:
> Hi Erik,
>
> Thanks for having a look.
>
>> A few comments:
>>
>> 1) Please make sure all files referencing these accessors that have been
>> moved to the oop.inline.hpp file now #include the oop.inline.hpp file. I
>> found that there are quite a few missing by looking for references to
>> oopDesc::mark() with rtags.
>> 2) I think the mark_addr() function should now be called mark_addr_raw()
>> for consistency.
>> 3) I wonder if there should be _raw() accessors that the GCs use
>> instead. It feels like the GC should not go through the Access API to
>> acquire mark words.
> You are of course right in all 3 points. Unfortunately, that causes
> massive ripples through the code base ;-)
>
> So here is what I did: I added _raw() variants for stuff that is used by
> GC. I temporarily renamed all relevant methods, e.g. mark() ->
> mark_fluff() to make the compiler help me. Then for each occurance of
> any such call decided whether it should call the usual method, or the
> raw variant (as a general rule, everything under src/hotspot/share/gc
> uses the _raw() variants). Also, I added #include "oop.inline.hpp" for
> every such occurance. Then I renamed all _fluff() methods and calls back
> to usual. Did this exercise with and without PCH, with and without zero.
> Checked with grep if I might have missed some.
>
> Notes: bytecodeInterpreter.cpp needed some additional fixes to use
> cas_set_mark() instead of the Atomic::cmpxchg().
>
> There are a bunch of methods in oop.hpp that are only used by GC, like
> is_locked() etc. I haven't changed them to _raw().
>
> This passes tier1 tests release+fastdebug, and as mentioned above builds
> with and without PCH and zero.
>
> Differential:
> http://cr.openjdk.java.net/~rkennke/JDK-8199735/webrev.03.diff/
> Full:
> http://cr.openjdk.java.net/~rkennke/JDK-8199735/webrev.03/
>
> Better now?
>
> Best regards,
> Roman
>
>
>
>> Thanks,
>> /Erik
>>
>>
>> On 2018-03-21 20:21, Roman Kennke wrote:
>>> Am 19.03.2018 um 12:07 schrieb Roman Kennke:
>>>> Am 19.03.2018 um 11:40 schrieb Roman Kennke:
>>>>> Currently, the mark word is accessed directly in oopDesc::mark()
>>>>> set_mark() and a bunch of other accessors. Those need to use the Access
>>>>> API instead because GC might want to employ barriers on those accesses,
>>>>> pretty much like every other field in oopDesc.
>>>>>
>>>>> Notice that this is *not* about accessing the bits and fields inside
>>>>> the
>>>>> markOop, but about accessing the header itself.
>>>>>
>>>>> http://cr.openjdk.java.net/~rkennke/JDK-8199735/webrev.01/
>>>>>
>>>>> Testing: build fastdebug/release with and without PCH, passed tier1
>>>>> fastdebug/release.
>>>>>
>>>>> Can I please get reviews?
>>>>>
>>>>> Thanks, Roman
>>>>>
>>>> Just when I sent it, I realized that this is dropping the volatile from
>>>> the mark word access. Fixed here:
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~rkennke/JDK-8199735/webrev.02/
>>>>
>>> Ping?
>>>
>