RFR: 8256302: releasing oopStorage when deflating allows for faster deleting

[Daniel D. Daugherty]

On Fri, 2 Dec 2022 07:35:40 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>>> You can ask by doing `_object().is_null()`
>> 
>> Are you allowed to do that when safepoint-safe?
>
>> > You can ask by doing `_object().is_null()`
>> 
>> Are you allowed to do that when safepoint-safe?
> 
> Yes, since we are only checking if we have pointer to an oop and not actually touching the oop it self.
> 
> oop* _obj;
> ...
> return _obj == NULL;

The sanity check code that @robehn proposed has an error:

 ObjectMonitor::~ObjectMonitor() {
+  if (UseNewCode) guarantee(_object.is_null(), "Destructor must never be called if weak handle is not released.");
 }


The `_object.is_null()` always returns false because the `_object` field is allocated
as a WeakHandle at ObjectMonitor construction time so it is never null.

The right code is this:

 ObjectMonitor::~ObjectMonitor() {
+  if (UseNewCode) guarantee(_object.peek() == nullptr, "Destructor must never be called if weak handle is not released.");
 }


but that code blows up with this PR because we are calling WeakHandle.peek() from a
thread state of `_thread_blocked` which results in this failure mode:

#  Internal Error (/System/Volumes/Data/work/shared/bug_hunt/8256302_for_jdk21.git/open/src/hotspot/share/oops/accessBackend.cpp:224), pid=87344, tid=23555
#  assert(state == _thread_in_vm || state == _thread_in_Java || state == _thread_new) failed: Wrong thread state for accesses: 10

<snip>

V  [libjvm.dylib+0x6ec95c]  report_vm_error(char const*, int, char const*, char const*, ...)+0x1ac
V  [libjvm.dylib+0x10098]  AccessInternal::check_access_thread_state()+0xb8
V  [libjvm.dylib+0x382bd1]  AccessInternal::RuntimeDispatch<593988ull, oopDesc*, (AccessInternal::BarrierType)2>::load(void*)+0x11
V  [libjvm.dylib+0x382b81]  std::__1::enable_if<!(HasDecorator<593988ull, AS_RAW>::value), oopDesc*>::type AccessInternal::PreRuntimeDispatch::load<593988ull, oopDesc*>(void*)+0x41
V  [libjvm.dylib+0x382b38]  oopDesc* AccessInternal::load_reduce_types<593988ull, oopDesc*>(oopDesc**)+0x18
V  [libjvm.dylib+0x382b02]  oopDesc* AccessInternal::load<593924ull, oopDesc*, oopDesc*>(oopDesc**)+0x22
V  [libjvm.dylib+0x382a38]  AccessInternal::OopLoadProxy<oopDesc*, 593920ull>::operator oopDesc*()+0x18
V  [libjvm.dylib+0x5f62f6]  WeakHandle::peek() const+0x76
V  [libjvm.dylib+0xfefcab]  ObjectMonitor::~ObjectMonitor()+0x3b
V  [libjvm.dylib+0xfefd05]  ObjectMonitor::~ObjectMonitor()+0x15
V  [libjvm.dylib+0x1279b74]  delete_monitors(GrowableArray<ObjectMonitor*>*)+0x84
V  [libjvm.dylib+0x12796c0]  ObjectSynchronizer::deflate_idle_monitors(ObjectMonitorsHashtable*)+0x510
V  [libjvm.dylib+0xfa0f6b]  MonitorDeflationThread::monitor_deflation_thread_entry(JavaThread*, JavaThread*)+0x12b


Remember, the key piece of this idea is that we want to delete the ObjectMonitors
from the MonitorDeflationThread while the thread is in `_thread_blocked` so that
we don't delay any safepoints.

If we do that, then we can't do even the simplest thing with the _object WeakHandle.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/11296#issuecomment-1572729642