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

[Daniel D.Daugherty]

On Mon, 14 Sep 2020 16:16:53 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>>> > Shenandoah changes are not complete:
>>> > /home/rkennke/src/openjdk/jdk/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp: In member function 'virtual
>>> > void ShenandoahFinalMarkingTask::work(uint)':
>>> > /home/rkennke/src/openjdk/jdk/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp:298:31: error: 'oops_do' is
>>> > not a member of 'ObjectSynchronizer' ObjectSynchronizer::oops_do(&resolve_mark_cl); ^~~~~~~
>>> > /home/rkennke/src/openjdk/jdk/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp:308:31: error: 'oops_do' is
>>> > not a member of 'ObjectSynchronizer' ObjectSynchronizer::oops_do(&mark_cl);
>>> > ^~~~~~~
>>> > I will have a look into how to resolve this.
>>> > In order to fix this, I need one of two things:
>>> > ```
>>> > * A way to iterate oops of the global-list. Explanation: In Shenandoah's I-U mode, we need to re-mark such ObjectMonitors
>>> >   during final-mark because they may be the only remaining reference to an object on the heap.
>>> >   _or_ (even better)
>>> > 
>>> > * Call a write-barrier (IN_NATIVE) whenever an ObjectMonitor is moved to the global-list upon thread-destruction. This
>>> >   way we could mark that object concurrently.
>>> > ```
>>> > 
>>> > 
>>> > Notice that this does not affect thread-local ObjectMonitors (IOW, most regular ObjectMonitors) because those would be
>>> > scanned while scanning thread stacks. Therefore I'd like to avoid to generally place a barrier in ObjectMonitor. See:
>>> > https://bugs.openjdk.java.net/browse/JDK-8251451 AFAICT, this may affect ZGC too (not 100% sure about this).
>>> 
>>> So this change makes the object monitors weak. So you shouldn't have to remark them at all. If they hold the last
>>> reference to the object, then the monitor gets the object cleared as part of normal weak OopStorage reference
>>> processing. Subsequent deflation cycles will detect that the GC has cleared the oop, and reuse the monitor. In other
>>> words, we should just remove the call to the oops_do function, and rely on OopStorage doing its thing. The GC should
>>> not have to care at all about monitors any longer, only about processing weak OopStorages, which is done automatically.
>>> Hope this makes sense.
>> 
>> Yes, definitely. I came to the same conclusion. Thank you!
>> With the patch amended:
>> http://cr.openjdk.java.net/~rkennke/8247281-shenandoah.patch
>> 
>> I'm good with the change.
>
> @rkennke - I've commited the changes in your webrev as
> https://github.com/openjdk/jdk/pull/135/commits/bbf8dbd09bdf5c1c77c67cc637fbc10fe72d4894.

@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. :-)

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

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