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

[Daniel D.Daugherty]

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

>> @dholmes-ora and @fisk - 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. :-)
>
> @kimbarrett:
> 
>> src/hotspot/share/oops/weakHandle.cpp
>> 36 WeakHandle::WeakHandle(OopStorage* storage, oop obj) :
>> 37 _obj(storage->allocate()) {
>> 38 assert(obj != NULL, "no need to create weak null oop");
>> 
>> Please format this differently so the ctor-init-list is more easily
>> distinguished from the body. I don't care that much which of the several
>> alternatives is used.
> 
> After discussion with Erik, I changed the indent on L37 from two space to four spaces.

@kimbarrett

> src/hotspot/share/runtime/objectMonitor.cpp
> 244 // Check that object() and set_object() are called from the right context:
> 245 static void check_object_context() {
> 
> This seems like checking we would normally only do in a debug build. Is this
> really supposed to be done in product builds too? (It's written to support
> that, just wondering if that's really what we want.) Maybe these aren't
> called very often so it doesn't matter? I also see that guarantee (rather
> than assert) is used a fair amount in this and related code.

I've changed check_object_context() to only be defined and called
when ASSERT is defined. I've also changed the guarantee() calls
to assert() calls.

I've done a couple of Mach5 Tier[1-8] test cycles on this code so I'm
no longer worried about this code or its callers in release bits.

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

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