RFR(S): 8222518: Remove unnecessary caching of Parker object in java.lang.Thread

[Daniel D. Daugherty]

On 4/24/19 3:12 AM, David Holmes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8222518
> webrev: http://cr.openjdk.java.net/~dholmes/8222518/webrev/

src/hotspot/share/classfile/javaClasses.cpp
     L1629:   macro(_park_blocker_offset,  k, "parkBlocker", 
object_signature, false);
         Line ends with a ';' and the previous last line did not. When the
         THREAD_FIELDS_DO macro is called, it is already followed by a ';':

         L1635:   THREAD_FIELDS_DO(FIELD_COMPUTE_OFFSET);
         L1640:   THREAD_FIELDS_DO(FIELD_SERIALIZE_OFFSET);

src/hotspot/share/classfile/javaClasses.hpp
     No comments.

src/hotspot/share/prims/unsafe.cpp
     No comments.

src/java.base/share/classes/java/lang/Thread.java
     No comments.

Thumbs up.  I don't need to see another webrev if you choose to remove
the ';' on L1629.

Dan

>
> The original implementation of Unsafe.unpark simply extracted the 
> JavaThread reference from the java.lang.Thread oop and if non-null 
> extracted the Parker instance from it and invoked unpark. This was 
> racy however as the native JavaThread could terminate at any time and 
> deallocate the Parker.
>
> That logic was fixed by JDK-6271298 which used of combination of 
> type-stable-memory "event" objects for the Parker, along with use of 
> the Threads_lock to obtain the initial reference to the Parker (from a 
> JavaThread guaranteed to be alive), together with caching the native 
> Parker pointer in a field of java.lang.Thread. Even though the native 
> thread may have terminated the Parker was still valid (even if 
> associated with a different thread) and the unpark at worst was a 
> spurious wakeup for that other thread.
>
> When JDK-8167108 introduced Thread Safe-Memory-Reclaimation (SMR) the 
> logic was updated to always use the safe mechanism - we grab a 
> ThreadsListHandle then check the cached field, else lookup the native 
> thread to see if it is alive and locate the Parker instance that way.
> With SMR the caching of the Parker pointer no longer serves any 
> purpose - we no longer have a lock-free use-the-cache path versus a 
> lock-using populate-the-cache path. With SMR we've already"paid" for 
> the ability to ensure the native thread can't terminate regardless of 
> whether we lookup the field from the java.lang.Thread or the 
> JavaThread. So we can simplify the code and save a little footprint by 
> removing the cache from java.lang.Thread:
>
>     /*
>      * JVM-private state that persists after native thread termination.
>      */
>     private long nativeParkEventPointer;
>
> and the supporting code from unsafe.cpp and javaClass.*pp in the JVM.
>
> I considered restoring the fast-path use of the cache without recourse 
> to Thread-SMR but performance measurements failed to show any benefit 
> in doing. See bug report for details.
>
> Thanks,
> David