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
