[15] RFR 8239926: Shenandoah: Shenandoah needs to mark nmethod's metadata

[Zhengyu Gu]

Overnight tests showed problems with this patch. So, I would like to 
withdraw this RFR.

Thanks,

-Zhengyu

On 3/12/20 12:43 PM, Roman Kennke wrote:
>>> Hi Zhengyu,
>>>
>>> in src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp:
>>> +      } else if
>>> (ShenandoahConcurrentRoots::can_do_concurrent_class_unloading()) {
>>> +        // Disarm nmethods that armed for concurrent mark.
>>> +        // On normal code path (non-empty Cset), it depends on
>>> update_roots() to
>>> +        // disarm nmethods in degenerated GC.
>>> +        ShenandoahCodeRoots::disarm_nmethods();
>>>
>>> beware that the update_roots() is only called at the end of update_refs
>>> phase. The same call at end of marking is orphaned since removal of
>>> piggy-backed marking.
>>
>> I think it is fine, successful degenerated GC cycle should always
>> execute update_refs, no?
> 
> Ok. I was only worried because the comment seems to imply it relies to
> update_roots() at the end of mark. Aleksey's patch is removing that. If
> update_roots() at the end of update_refs is good too, then fine.
> 
> Thanks,
> Roman
> 
>> Thanks,
>>
>> -Zhengyu
>>
>>>
>>> Otherwise looks good.
>>>
>>> Thanks,
>>> Roman
>>>
>>>
>>> On 3/11/20 10:43 PM, Zhengyu Gu wrote:
>>>> Revised based on offline discussions.
>>>>
>>>> Piggyback on stack code root rescanning to SATB draining task.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8239926/webrev.02/
>>>>
>>>> Reran tests:
>>>>     hotspot_gc_shenandoah
>>>>     tools/javac
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>>
>>>> On 3/4/20 6:06 PM, Zhengyu Gu wrote:
>>>>> Traversal GC has the same issue, also need to remark on stack code
>>>>> roots in final traversal.
>>>>>
>>>>> @@ -263,11 +263,12 @@
>>>>>         if (!_heap->is_degenerated_gc_in_progress()) {
>>>>>           ShenandoahTraversalRootsClosure roots_cl(q, rp);
>>>>>           ShenandoahTraversalSATBThreadsClosure tc(&satb_cl);
>>>>>           if (unload_classes) {
>>>>>             ShenandoahRemarkCLDClosure remark_cld_cl(&roots_cl);
>>>>> -        _rp->strong_roots_do(worker_id, &roots_cl, &remark_cld_cl,
>>>>> NULL, &tc);
>>>>> +        MarkingCodeBlobClosure code_cl(&roots_cl,
>>>>> CodeBlobToOopClosure::FixRelocations);
>>>>> +        _rp->strong_roots_do(worker_id, &roots_cl, &remark_cld_cl,
>>>>> &code_cl, &tc);
>>>>>           } else {
>>>>>             CLDToOopClosure cld_cl(&roots_cl,
>>>>> ClassLoaderData::_claim_strong);
>>>>>             _rp->roots_do(worker_id, &roots_cl, &cld_cl, NULL, &tc);
>>>>>           }
>>>>>         } else {
>>>>>
>>>>>
>>>>> Updated webrev: http://cr.openjdk.java.net/~zgu/JDK-8239926/webrev.01/
>>>>>
>>>>> Thank,
>>>>>
>>>>> -Zhengyu
>>>>>
>>>>> On 2/25/20 12:13 PM, Zhengyu Gu wrote:
>>>>>> Shenandoah encounters a few test failures with tools/javac. Verifier
>>>>>> catches unmarked oops in nmethod's metadata during root evacuation in
>>>>>> final mark phase.
>>>>>>
>>>>>> The problem is that, Shenandoah marks on stack nmethods in init mark
>>>>>> pause, but it does not mark nmethod's metadata during concurrent mark
>>>>>> phase, when new nmethod is about to be executed.
>>>>>>
>>>>>> The solution:
>>>>>> 1) Use nmethod_entry_barrier to keep nmethod's metadata alive when
>>>>>> the nmethod is about to be executed, when nmethod entry barrier is
>>>>>> supported.
>>>>>>
>>>>>> 2) Remark on stack nmethod's metadata at final mark pause.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8239926
>>>>>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8239926/webrev.00/
>>>>>>
>>>>>> Test:
>>>>>>      hotspot_gc_shenandoah (fastdebug and release)
>>>>>>      tools/javac with ShenandoahCodeRootsStyle = 1 and 2 (fastdebug and
>>>>>> release)
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Zhengyu
>>>>
>>>
>>
>