RFR: 8242054: Shenandoah: New incremental-update mode

[Roman Kennke]

I made a mistake with webrev.02, it included the change for JDK-8242130.
Please review this webrev instead:

http://cr.openjdk.java.net/~rkennke/JDK-8242054/webrev.03/

Thanks,
Roman


>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8242054
>>> Webrev:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8242054/webrev.00/
> 
> I rebased this patch on top of "JDK-8242130: Shenandoah: Simplify
> arraycopy-barrier dispatching"
> 
> Now that we purged traversal-mode, I also added tests for IU mode, by
> (mostly) mechanically adding back the tests that I removed with
> Traversal mode, and change ShenandoahGCMode=traversal ->
> ShenandoahGCMode=iu.
>  	
>> *) I would prefer for flag checks to be first. It makes it clearer to me that path is only taken in
>> some configurations. Example, this:
>>   if (dest_uninitialized && ShenandoahSATBBarrier) ...
>>  ...becomes this:
>>   if (ShenandoahSATBBarrier && dest_uninitialized) ...
> 
> Right. Fixed the occurances that I found (the mentioned one is gone with
> JDK-8242130.
> 
>> *) These blocks should not be removed. At very least we want to make the check for SATB/StoreVal
>> barrier conditional on the mode. But other barrier checks should definitely stay.
>>
>>   47   // Final configuration checks
>>   48   SHENANDOAH_CHECK_FLAG_SET(ShenandoahLoadRefBarrier);
>>   49   SHENANDOAH_CHECK_FLAG_SET(ShenandoahSATBBarrier);
>>   50   SHENANDOAH_CHECK_FLAG_SET(ShenandoahCASBarrier);
>>   51   SHENANDOAH_CHECK_FLAG_SET(ShenandoahCloneBarrier);
> 
> Ok. I turned it into:
> SHENANDOAH_CHECK_FLAG_SET(ShenandoahSATBBarrier ||
> ShenandoahStoreValEnqueueBarrier)
> 
> which required the macro to do !(name) instead of !name.
> 
>> *) This is some local madness:
>>
>>   84   int gc_state = _heap->gc_state();
>>   85   if ((gc_state & ShenandoahHeap::MARKING) != 0 &&
>>   86       ShenandoahStoreValEnqueueBarrier &&
>>   87       !_heap->marking_context()->allocated_after_mark_start(obj)) {
>>   88     ShenandoahUpdateRefsForOopClosure<false, false, true> cl;
>>   89     obj->oop_iterate(&cl);
>>   90   } else if ((gc_state & ShenandoahHeap::HAS_FORWARDED) != 0) {
>>   91     if (skip_bulk_update(cast_from_oop<HeapWord *>(obj))) return;
> 
> This should be gone with JDK-8242130, plus in this revision I also made
> the clone_barrier() to dispatch in a very similar fashion.
> 
>> *) I don't think the rewrite in ShFinalMarkTask is equivalent. See, on the path here:
>>
>>  262 ShenandoahSATBAndRemarkCodeRootsThreadsClosure tc(&cl,
>>  263                                     ShenandoahStoreValEnqueueBarrier ? &mark_cl : NULL,
>>  264                                     &blobsCl);
>>
>>  ...we used to pass NULL as blobsCL when do_nmethods is false.
> 
> Right. Good catch!
> 
>> *) Also, why ResourceMark here?
>>
>>  199         if (_cl != NULL) {
>>  200           ResourceMark rm;
>>  201           thread->oops_do(_cl, _code_cl);
> 
> Because Threads::oops_do() allocates some Resource stuff. It's
> everywhere where Thread::oops_do() is called. Should probably be
> looked-at separately?
> 
> 
> http://cr.openjdk.java.net/~rkennke/JDK-8242054/webrev.02/
> 
> Better?
> 
> Roman
>