On Mon, 14 Sep 2020 18:33:17 GMT, Daniel D. Daugherty <dcu...@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
