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

[Roman Kennke]

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