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

[David Holmes]

I pushed this today based on Dan and Robbin's reviews, but realized just 
after the act that I should have waited for any feedback from core-libs 
- apologies about that. If there are concerns I will roll it back.

Thanks,
David
-----

On 26/04/2019 2:57 pm, David Holmes wrote:
> Thanks Dan! Extraneous ; culled.
> 
> David
> 
> On 25/04/2019 1:16 am, Daniel D. Daugherty wrote:
>> 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
>>