RFR: Concurrent partial should fix up roots only on cancel

[Aleksey Shipilev]

Actually, the change in Verifier path in incorrect, correct change is:

diff -r 5722209e1105 src/share/vm/gc/shenandoah/shenandoahPartialGC.cpp
--- a/src/share/vm/gc/shenandoah/shenandoahPartialGC.cpp	Tue Oct 10 13:24:55 2017 +0200
+++ b/src/share/vm/gc/shenandoah/shenandoahPartialGC.cpp	Tue Oct 10 15:33:05 2017 +0200
@@ -527,7 +527,9 @@
     if (ShenandoahVerify && ! _heap->cancelled_concgc()) {
       _heap->verifier()->verify_after_partial();
     }
-  } else {
+  }
+
+  if (_heap->cancelled_concgc()) {
     // Fixup roots to make them consistent
     _heap->fixup_roots();
   }

-Aleksey

On 10/10/2017 03:30 PM, Zhengyu Gu wrote:
> Good to me.
> 
> -Zhengyu
> 
> On 10/10/2017 09:04 AM, Aleksey Shipilev wrote:
>> http://cr.openjdk.java.net/~shade/shenandoah/concpartial-fixroots-cancel/webrev.01/
>>
>> Obvious when you stare into assert like:
>>
>> #  assert(cancelled_concgc()) failed: Only after concurrent cycle failed
>>
>> ...because the condition in final_partial_collection is cunning:
>>
>>   476   if (_has_work && ! _heap->cancelled_concgc()) {
>>
>> ...can call into "else" branch, and thus call heap->fixup_roots() without cancelled GC, when
>> _has_work=false. This will fail the assert inside fixup_roots(). We should just do that on
>> cancellation path only.
>>
>> Testing: hotspot_gc_shenandoah
>>
>> Thanks,
>> -Aleksey
>>