RFR 8224875: Shenandoah: ParallelCleaning code unloading should take lock to protect shared code roots array

[Zhengyu Gu]

On 5/28/19 12:28 PM, Aleksey Shipilev wrote:
> On 5/28/19 6:17 PM, Zhengyu Gu wrote:
>> The patch changes ShenandoahHeapLock to general purpose spin lock and uses it to protect concurrent
>> access to shared array during parallel cleaning at safepoints.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8224875
>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8224875/webrev.00/
> 
> *) Name it ShenandoahLock, maybe? The fact it is a spin lock is the implementation detail. Maybe we
> actually want to split out the rename into a separate changeset, so we can backport it ahead of time.

Okay, split into JDK-8224932


> 
> *) These two typedefs deserve to be in shenandoahHeap.hpp?
> 
>    96 typedef ShenandoahSpinLock   ShenandoahHeapLock;
>    97 typedef ShenandoahSpinLocker ShenandoahHeapLocker;

Done in JDK-8224932

> 
> *) You don't need a comment here, I think it is clear what that lock protects
> 
>   136   // Lock to protect recorded nms array
>   137   static ShenandoahSpinLock                 _recorded_nms_lock;

Okay.

> 
> *) Since you moved the assert in ShenandoahCodeRoots::remove_nmethod, is it worth it to move it in
> ShenandoahCodeRoots::add_nmethod too?
> 
Sure.

Updated webrev: http://cr.openjdk.java.net/~zgu/JDK-8224875/webrev.01/

Reran hotspot_gc_shenandoah test.

Thanks,

-Zhengyu