RFR: Remove racy assert in ShenandoahResetNextMarkBitmapTraversalTask

[Zhengyu Gu]

On 09/11/2018 08:06 AM, Roman Kennke wrote:
> The assert in ShenandoahResetNextMarkBitmapTraversalTask that is racily
> checking TAMS against top. Racy because other manipulations (e.g.
> recycle-assist) happen under HeapLock, but not this check. This
> sometimes blows up. I don't know how to fix that except removing it. We
> could acquire the HeapLock under #ifdef ASSERT but that seems like overkill.
> 
> 
Hmmm... I am not sure this ShenandoahResetNextBitmapTraversalTask is 
safe at all.

The region status races against mutator's allocation, e.g. 
region->is_trash() can race against FreeSet's recycle_trash(), now you 
have memory ordering issues to worry about. Am I wrong?

Thanks,

-Zhengyu

> With this patch, aarch64 passes tier3_gc_shenandoah
> 
> Notice that the whole thing is still somewhat racy, but seems
> conservatively ok. Getting this right without serializing the
> reset-bitmaps-stuff would require to use per-region-locks for this sort
> of thing.
> 
> Ok?
> 
> Roman
> 
> 
> # HG changeset patch
> # User rkennke
> # Date 1536667335 -7200
> #      Tue Sep 11 14:02:15 2018 +0200
> # Node ID 088a07bcf78aa6b020b7cc1c60231bb2dacc39b7
> # Parent  a391e22f381ea337ed647b4da4fd3c26def6c03f
> Remove racy assert in ShenandoahResetNextMarkBitmapTraversalTask
> 
> diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
> b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
> --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
> @@ -471,9 +471,6 @@
>           assert(heap->is_bitmap_slice_committed(region), "sanity");
>           HeapWord* bottom = region->bottom();
>           HeapWord* top =
> next_ctx->top_at_mark_start(region->region_number());
> -        assert(top <= region->top(),
> -               "TAMS must smaller/equals than top: TAMS: "PTR_FORMAT",
> top: "PTR_FORMAT,
> -               p2i(top), p2i(region->top()));
>           if (top > bottom) {
> 
> compl_ctx->mark_bit_map()->copy_from(next_ctx->mark_bit_map(),
> MemRegion(bottom, top));
>             compl_ctx->set_top_at_mark_start(region->region_number(), top);
>