RFR 8227103: Shenandoah: Refactor ShenandoahNMethod in preparation of concurrent nmethod iteration

[Zhengyu Gu]

Ping!

Thanks,

-Zhengyu

On 7/23/19 11:40 AM, Zhengyu Gu wrote:
> Updated: http://cr.openjdk.java.net/~zgu/JDK-8227103/webrev.01/
> 
> On 7/3/19 4:47 AM, Aleksey Shipilev wrote:
>> On 7/2/19 6:51 PM, Zhengyu Gu wrote:
>>> This refactor is one of subtasks for concurrent nmethod unloading [1].
>>>
>>> It moves ShenandoahNMethod out of ShenandoahCodeRoots, in preparation 
>>> for more complicated
>>> concurrent nmethod iteration.
>>>
>>> Borrowing what ZGC does, only records oops embedded in relocation 
>>> stream to reduce memory footprint.
>>> Also, attaches ShenandoahNMethod to nmethod's gc_data, so that it 
>>> avoids additional searching during
>>> re-register nmethod.
>>>
>>> Let's use shenandoah/jdk for concurrent nmethod unloading work, while 
>>> we stabilize early concurrent
>>> root work in jdk/jdk.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8227103
>>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8227103/webrev.00/
>>
>> I have several design concerns here:
>>
>>   *) The ability to attach binary blob data to nmethod only exists 
>> since JDK 12 (JDK-8214056), which
>> would break the backporting. Current code deliberately does its own 
>> thing, so that it works everywhere.
> 
> Dropped attaching ShenandoahNMethod to nmethod to maintain backportability.
> 
>>
>>   *) If you use nmethod data, you need to implement 
>> CollectedHeap::flush_nmethod that would delete
>> ShenandoahNMethod, otherwise this would leak memory.
>>
>>   *) Scanning relocation stream only might be an interesting thing to 
>> do, but I think the performance
>> improvements we would get from this does not justify the risks of 
>> avoiding "proper" oops_do,
>> especially after code roots are scanned concurrently.
> 
> Added assert to ensure the results are match with 
> ShenandoahNMethodOopDetector.
> 
> Reran hotspot_gc_shenandoah (fastdebug and release)
> 
> Thanks,
> 
> -Zhengyu
> 
> 
>>
>> Can you say what exactly do you need to enable "more complicated 
>> concurrent nmethod iteration"?
>>
>> -Aleksey
>>