RFR (s): 8250606: Remove unnecessary assertions in ObjectSynchronizer FastHashcode and inflate

David Holmes david.holmes at oracle.com
Wed Aug 5 07:58:31 UTC 2020


Bug: https://bugs.openjdk.java.net/browse/JDK-8250606
webrev: http://cr.openjdk.java.net/~dholmes/8250606/webrev/

tl;dr I removed some assertions that were deemed unnecessary after 
interfering with other work. :)

ObjectSynchronizer::inflate had the following assertion added back in 
JDK 6 under JDK-5030359:

ObjectMonitor * ObjectSynchronizer::inflate (oop object) {
   // Inflate mutates the heap ...
   assert (!SafepointSynchronize::is_at_safepoint(), "invariant") ;

That same change added FastHashcode which had the following assertions:

intptr_t ObjectSynchronizer::FastHashCode (Thread * Self, oop obj) {
   assert (!SafepointSynchronize::is_at_safepoint(), "invariant") ;
   assert (Self->is_Java_thread(), "invariant") ;
   assert (((JavaThread *)Self)->thread_state() != _thread_blocked, 
"invariant") ;

These methods had assertions that they were not executed at a safepoint 
in case the modification of the markWord could cause interference with GC.

JDK-6320749 then relaxed the assertions to allow for GC verification - 
which does happen at a safepoint and so obviously not in a JavaThread.

The FastHashCode assertions were further relaxed in JDK-8015086 to allow 
the CDS dump VM safepoint operation to call it. But FastHashCode can 
also call inflate so that was a mismatch in assertions just waiting to 
happen - which is how I hit this in a VM that always forces inflation :)

I initially intended to extend the FastHashCode assertion to match 
inflate, but then a question arose about the safety of the CDS dumping 
code using FastHashcode which showed that the code is in fact safe. 
Notwithstanding what the state of the world may have been in the JDK 6 
timeframe, there is now no potential interference possible between 
execution of these methods at an arbitrary safepoint and any concurrent 
GC use of the markWord. So the original !is_at_safepoint assertion can 
just be dropped from both methods. That in turn means we either drop the 
JavaThread assertion or else expand it to be "JavaThread or VMThread" - 
but that itself seems unnecessarily restrictive: why can't a GC thread 
use this code for example? So I chose to drop it.

That leaves the "if I'm a JavaThread I'm not _thread_blocked" assertion. 
I don't see that this really guards against anything useful (a 
JavaThread should be _thread_in_vm when executing this code ... though 
there may be leaf calls where the thread is _thread_in_java). So again I 
just dropped the assertion.

So a long winded justification for dropping a few assertions. :)

Thanks,
David




More information about the hotspot-runtime-dev mailing list