RFR: 8140584: nmethod::oops_do_marking_epilogue always runs verification code

[Stefan Karlsson]

On 2015-11-09 18:54, Vladimir Kozlov wrote:
> Very nice.

Thanks.

> You may need closed changes too.

You're right.

Thanks.
StefanK

>
> Thanks,
> Vladimir
>
> On 11/9/15 1:34 AM, Stefan Karlsson wrote:
>> Hi Vladimir,
>>
>> What do you think about this version:
>>   http://cr.openjdk.java.net/~stefank/8140584/webrev.02/
>>   http://cr.openjdk.java.net/~stefank/8140584/webrev.02.delta/
>>
>> Thanks,
>> StefanK
>>
>> On 2015-10-30 09:44, Vladimir Kozlov wrote:
>>> On 10/29/15 11:52 PM, Stefan Karlsson wrote:
>>>> Hi Vladimir,
>>>>
>>>> On 2015-10-27 16:22, Vladimir Kozlov wrote:
>>>>> CodeCache::verify_oops() also calls it unguarded.
>>>>>
>>>>> I think for complete solution nmethod::verify_oop_relocations() and
>>>>> called from it oop_Relocation::verify_oop_relocation(),
>>>>> verify_value(), const_verify_data_value() and pd_verify_data_value()
>>>>> should be declared and defined in debug build only.
>>>>
>>>> I see what you're saying, but I'm unsure that this is really what we
>>>> want to do.
>>>>
>>>> CodeCache::verify_oops is called from the Universe::verify, that 
>>>> can be
>>>> enabled in product builds by setting -XX:+UnlockDiagnosticVMOptions 
>>>> and
>>>> -XX:+VerifyBeforeGC and/or -XX:+VerifyAfterGC. Maybe we want to change
>>>> the relocation code to use guarantees instead of asserts? So that 
>>>> we can
>>>> get verification of the relocation code in product builds?
>>>
>>> Yes, I think it is good suggestion (asserts -> guarantees or fatals) 
>>> in addition to webrev.01.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>
>>>> Thanks,
>>>> StefanK
>>>>
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 10/27/15 7:44 PM, Stefan Karlsson wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review this patch to remove some unnecessary verification 
>>>>>> code
>>>>>> from our product builds.
>>>>>>
>>>>>> Today, CodeCache::gc_epilogue() guards the call to
>>>>>> nmethod::verify_oop_relocations with DEBUG_ONLY, while
>>>>>> nmethod::oops_do_marking_epilogue doesn't. Since
>>>>>> nmethod::verify_oop_relocations uses asserts when verifying the
>>>>>> relocations, this is just wasted cycles in product builds. This 
>>>>>> patch
>>>>>> adds DEBUG_ONLY to nmethod::oops_do_marking_epilogue.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~stefank/8140584/webrev.01/
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8140584
>>>>>>
>>>>>> Thanks,
>>>>>> StefanK
>>>>
>>