RFR: 8247281: migrate ObjectMonitor::_object to OopStorage [v2]

[Daniel D.Daugherty]

On Fri, 11 Sep 2020 21:10:27 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> src/hotspot/share/runtime/objectMonitor.cpp line 251:
>> 
>>> 249:   guarantee(self->is_Java_thread() || self->is_VM_thread(), "must be");
>>> 250:   if (self->is_Java_thread()) {
>>> 251:     JavaThread* jt = (JavaThread*)self;
>> 
>> With David's new change this should use as_Java_thread().
>
> Yup. Since this is a new function, it didn't pop up as a conflict.
> I'll fix that.

Fixed in https://github.com/openjdk/jdk/pull/135/commits/750fe771943178a02f1b71a713f7417512e3628e.

>> src/hotspot/share/runtime/objectMonitor.cpp line 246:
>> 
>>> 244: // Check that object() and set_object() are called from the right context:
>>> 245: static void check_object_context() {
>>> 246:   Thread *self = Thread::current();
>> 
>> Nit * is in the wrong place.
>
> I'll fix that.

Fixed in https://github.com/openjdk/jdk/pull/135/commits/750fe771943178a02f1b71a713f7417512e3628e

>> src/hotspot/share/runtime/synchronizer.cpp line 1548:
>> 
>>> 1546:                                     bool from_per_thread_alloc) {
>>> 1547:   guarantee(m->header().value() == 0, "invariant");
>>> 1548:   guarantee(m->object_peek() == NULL, "invariant");
>> 
>> Because of type stable memory, you don't release the WeakHandle when the OM is released.  The oop inside the WeakHandle
>> is replaced in when an OM is reused?
>
> Correct. See:
> 
> +void ObjectMonitor::set_object(oop obj) {
> +  check_object_context();
> +  if (_object.is_null()) {
> +    _object = WeakHandle(_oop_storage, obj);
> +  } else {
> +    _object.replace(obj);
> +  }
> +}
> 
> So when 'obj' == NULL, we replace the weak handle's value with NULL
> and we don't release/delete/whatever the weak handle.

@fisk  - I can't figure out what "magic" to use to get the above code quote to show up as such. Suggestion?

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

PR: https://git.openjdk.java.net/jdk/pull/135