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

[Aleksey Shipilev]

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.

*) These two typedefs deserve to be in shenandoahHeap.hpp?

  96 typedef ShenandoahSpinLock   ShenandoahHeapLock;
  97 typedef ShenandoahSpinLocker ShenandoahHeapLocker;

*) 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;

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

-- 
Thanks,
-Aleksey