RFR(S) 228777: Shenandoah: Cleanup STW weak root processing
Shenandoah pre-evacuates or updates weak roots at STW pauses. Therefore, weak roots should not have forwarded references during mark phase. Current has_forwarded_objects() branch does not break things, but does unncessary works, let's prune it. Bug: https://bugs.openjdk.java.net/browse/JDK-8228777 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8228777/webrev.00/ Test: hotspot_gc_shenandoah (fastdebug and release) Thanks, -Zhengyu
I am not sure about this. If final-mark could not finish to pre-evacuate all roots we might have from-space refs there. Or does the OOM-protocol ensure that any ref should be updated correctly? We need to be careful here. Better safe than sorry. Roman
Shenandoah pre-evacuates or updates weak roots at STW pauses. Therefore, weak roots should not have forwarded references during mark phase.
Current has_forwarded_objects() branch does not break things, but does unncessary works, let's prune it.
Bug: https://bugs.openjdk.java.net/browse/JDK-8228777 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8228777/webrev.00/
Test: hotspot_gc_shenandoah (fastdebug and release)
Thanks,
-Zhengyu
On 7/30/19 12:50 PM, Roman Kennke wrote:
I am not sure about this. If final-mark could not finish to pre-evacuate all roots we might have from-space refs there. Or does the OOM-protocol ensure that any ref should be updated correctly? We need to be careful here. Better safe than sorry.
If pre-evacuation fails, it runs degenerated GC cycle and roots are updated in final_updaterefs(). We do have no-forwarded assertion in place, should catch the problem if otherwise. Thanks, -Zhengyu
Roman
Shenandoah pre-evacuates or updates weak roots at STW pauses. Therefore, weak roots should not have forwarded references during mark phase.
Current has_forwarded_objects() branch does not break things, but does unncessary works, let's prune it.
Bug: https://bugs.openjdk.java.net/browse/JDK-8228777 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8228777/webrev.00/
Test: hotspot_gc_shenandoah (fastdebug and release)
Thanks,
-Zhengyu
On 7/30/19 1:26 PM, Zhengyu Gu wrote:
On 7/30/19 12:50 PM, Roman Kennke wrote:
I am not sure about this. If final-mark could not finish to pre-evacuate all roots we might have from-space refs there. Or does the OOM-protocol ensure that any ref should be updated correctly? We need to be careful here. Better safe than sorry.
Actually, you are right. It does not work with traversal. I wonder how the tests passed early. I would like to withdraw this RFR. Thanks, -Zhengyu
If pre-evacuation fails, it runs degenerated GC cycle and roots are updated in final_updaterefs().
We do have no-forwarded assertion in place, should catch the problem if otherwise.
Thanks,
-Zhengyu
Roman
Shenandoah pre-evacuates or updates weak roots at STW pauses. Therefore, weak roots should not have forwarded references during mark phase.
Current has_forwarded_objects() branch does not break things, but does unncessary works, let's prune it.
Bug: https://bugs.openjdk.java.net/browse/JDK-8228777 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8228777/webrev.00/
Test: hotspot_gc_shenandoah (fastdebug and release)
Thanks,
-Zhengyu
Hi Roman, On 7/30/19 4:31 PM, Zhengyu Gu wrote:
On 7/30/19 1:26 PM, Zhengyu Gu wrote:
On 7/30/19 12:50 PM, Roman Kennke wrote:
I am not sure about this. If final-mark could not finish to pre-evacuate all roots we might have from-space refs there. Or does the OOM-protocol ensure that any ref should be updated correctly? We need to be careful here. Better safe than sorry.
Actually, you are right. It does not work with traversal. I wonder how the tests passed early.
I would like to withdraw this RFR.
It is not true. Traversal fixes forwarding before processes weak roots, so this patch should work. The failed tests were due to broken builds, they are passed on clean builds, also specjvm tests (normal and traversal mode). Therefore, I would like re-open this RFR. This invariant is important for implementing concurrent weak root processing (JDK-8228818). If you still have concerns, let's push it to shenandoah/jdk, Okay? Thanks, -Zhengyu
Thanks,
-Zhengyu
If pre-evacuation fails, it runs degenerated GC cycle and roots are updated in final_updaterefs().
We do have no-forwarded assertion in place, should catch the problem if otherwise.
Thanks,
-Zhengyu
Roman
Shenandoah pre-evacuates or updates weak roots at STW pauses. Therefore, weak roots should not have forwarded references during mark phase.
Current has_forwarded_objects() branch does not break things, but does unncessary works, let's prune it.
Bug: https://bugs.openjdk.java.net/browse/JDK-8228777 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8228777/webrev.00/
Test: hotspot_gc_shenandoah (fastdebug and release)
Thanks,
-Zhengyu
I am not sure about this. If final-mark could not finish to pre-evacuate all roots we might have from-space refs there. Or does the OOM-protocol ensure that any ref should be updated correctly? We need to be careful here. Better safe than sorry.
Actually, you are right. It does not work with traversal. I wonder how the tests passed early.
I would like to withdraw this RFR.
It is not true. Traversal fixes forwarding before processes weak roots, so this patch should work.
The failed tests were due to broken builds, they are passed on clean builds, also specjvm tests (normal and traversal mode). Therefore, I would like re-open this RFR. This invariant is important for implementing concurrent weak root processing (JDK-8228818).
If you still have concerns, let's push it to shenandoah/jdk, Okay?
Yeah, let's push it to sh/jdk and if it's ok, take it upstream to jdk/jdk. Thanks, Roman
Thanks,
-Zhengyu
Thanks,
-Zhengyu
If pre-evacuation fails, it runs degenerated GC cycle and roots are updated in final_updaterefs().
We do have no-forwarded assertion in place, should catch the problem if otherwise.
Thanks,
-Zhengyu
Roman
Shenandoah pre-evacuates or updates weak roots at STW pauses. Therefore, weak roots should not have forwarded references during mark phase.
Current has_forwarded_objects() branch does not break things, but does unncessary works, let's prune it.
Bug: https://bugs.openjdk.java.net/browse/JDK-8228777 Webrev: http://cr.openjdk.java.net/~zgu/JDK-8228777/webrev.00/
Test: hotspot_gc_shenandoah (fastdebug and release)
Thanks,
-Zhengyu
participants (2)
-
Roman Kennke
-
Zhengyu Gu