RFR 8232747: Shenandoah: Concurrent GC should deactivate SATB before processing weak roots

[Zhengyu Gu]

On 10/22/19 10:46 AM, Aleksey Shipilev wrote:
> On 10/22/19 4:31 PM, Zhengyu Gu wrote:
>> Updated: http://cr.openjdk.java.net/~zgu/JDK-8232747/webrev.01/index.html
> 
> Right. Looks much better. Still, a few nits:
> 
> *) We don't need to assert these anymore (we never do in other places)
> 
> 1481     assert(is_concurrent_mark_in_progress(), "How else could we get here?");
> ...
> 1585     assert(is_concurrent_mark_in_progress(), "How else could we get here?");
> 
> *) Newline between lines here, also captialize "Marking..."
> 
> 1479     concurrent_mark()->finish_mark_from_roots(/* full_gc = */ false);
> 1480     // marking is completed, deactivate SATB barrier
> 
> *) This is still awkwardly worded, that's my fault. Let's do this:
> 
>       concurrent_mark()->cancel();
>       assert(is_concurrent_mark_in_progress(), "How else could we get here?");
>       set_concurrent_mark_in_progress(false);
> 
>       // If this cycle was updating references, we need to keep the has_forwarded_objects
>       // flag on, for subsequent phases to deal with it.
> 
>       if (process_references())

All fixed and pushed.

> 
> *) You tested hotspot_gc_shenandoah to verify that adding parallel_cleaning call in mark-compact
> phase1 is safe, right?

Of course. And reran the tests after every iteration.

Thanks,

-Zhengyu

> 
> Otherwise looks good.
>