RFR (XS): 8200362: G1Mux2Closure should disable implicit oop verification

[Stefan Johansson]

On 2018-03-29 11:20, Stefan Karlsson wrote:
>
>
> On 2018-03-29 11:02, Thomas Schatzl wrote:
>> Hi Stefan,
>>
>> On Thu, 2018-03-29 at 09:52 +0200, Stefan Johansson wrote:
>>> Hi Thomas,
>>>
>>> On 2018-03-28 17:27, Thomas Schatzl wrote:
>>>> Hi all,
>>>>
>>>>     can I have reviews for this small change that fixes some
>>>> annoyance
>>>> when debugging with G1?
>>>>
>>>> In particular, G1Mux2Closure is used by verification to verify and
>>>> in
>>>> case of error print out g1 specific error messages.
>>>>
>>>> However, since G1Mux2Closure is an OopClosure, it is actually
>>>> wrapped
>>>> by NoHeaderExtendedOopClosure in oop_iterate* calls.
>>>> ExtendedOopClosure
>>>> has its own verification that is very generic, and may trigger in
>>>> the
>>>> same situations as G1Mux2Closure.
>>>>
>>>> Disable this implicit verification for G1Mux2Closure so that we
>>>> always
>>>> get the g1 specific error messages.
>>>>
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8200362
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~tschatzl/8200362/webrev/
>>>
>>> Nice change, always good to get detailed information. I think you
>>> need
>>> to change the code using the G1Mux2Closure as well. On line 666 in
>>> heapRegion.cpp:
>>>     G1Mux2Closure mux(&vl_cl, &vr_cl);
>>>     obj->oop_iterate_no_header(&mux);
>>> This should now be a normal oop_iterate(&mux) to make sure the right
>>> verification is done.
>>
>> Fixed. Also looked through other closures used for verification.
>>
>> http://cr.openjdk.java.net/~tschatzl/8200362/webrev.0_to_1 (diff)
>> http://cr.openjdk.java.net/~tschatzl/8200362/webrev.1 (full)
>
> Looks good.
>
> I like that we get rid of usages of oop_iterate_no_header. Maybe we 
> should remove the other usages in G1, as a separate RFE.
>
I like the sound of that, maybe something for Leo to look at. Any 
history or reason why we have these?

StefanJ
> StefanK
>
>>
>> Thomas
>>