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

[Vladimir Kozlov]

Very nice. You may need closed changes too.

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