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

[Daniel D.Daugherty]

On Mon, 14 Sep 2020 14:27:51 GMT, Roman Kennke <rkennke 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.
>
>> > 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.

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

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