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

[Daniel D.Daugherty]

On Mon, 14 Sep 2020 18:51:31 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> @kimbarrett
>> 
>>> src/hotspot/share/runtime/synchronizer.cpp
>>> 
>>> The old code in chk_in_use_entry seems wrong. It checked for a null
>>> object() and recorded that as an error. But then it went on and attempted
>>> to use it as if it was not null. That's been fixed by the change. However,
>>> the change no longer treats a null as an error. Probably this is because
>>> it's weak, and so could have become null. But is that really possible for
>>> an "in use" monitor?
>> 
>> In the old code, an ObjectMonitor's object field should never be NULL when
>> that ObjectMonitor is on an in-use list. We'll get the logging message and
>> then a crash. I used to have guarantee(n->object() != NULL, ...) in there,
>> but Robbin convinced me that was a waste because we'll just crash on
>> the use of the NULL pointer and that was good enough.
>> 
>> As Erik has already explained, now that we use a weak handle, the object
>> can be GC'ed before the deflater thread comes along and removes the
>> idle ObjectMonitor from the in-use list.
>
> @kimbarrett - I believe I've addressed your comments in this push:
> https://github.com/openjdk/jdk/pull/135/commits/9fa2bed109d6fe352c610b26ada650226ce9cec4

@coleenp, @rkennke, and @kimbarrett - I believe all of the changes that you have requested
have been made. Please confirm by re-reviewing.

@dholmes-ora - I don't think you asked for any specific code changes. I've taken a first pass
at creating a CSR:
JDK-8253121 migrate ObjectMonitor::_object to OopStorage
https://bugs.openjdk.java.net/browse/JDK-8253121

Please look it over and feel free to edit as needed. Since I don't do
CSR's often, what I've done might be all wrong. :-)

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

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