RFR: JDK-8221751: Shenandoah: Improve SATB enqueueing

Roman Kennke rkennke at redhat.com
Tue Apr 2 10:38:56 UTC 2019


>> In some paths, we currently check GC state 3 times before enqueueing an oop in SATB buffer: 1.
>> conc-mark in progress (global or local), 2. SATB active (global) 3. SATB active (local). This should
>> be streamlined to only do one check, and consistently check conc-mark-in-progress only.
>>
>> This can be taken even further for arraycopy pre-barriers and arraycopy-loops, where the check can
>> be done once for the whole operation. In arraycopy barriers code, we can also 'inline' the current
>> thread and marking context objects, instead of fetching them for every array element.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8221751
>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/JDK-8221751/webrev.00/
> 
> Looks okay.
> 
> *) In ShenandoahBarrierSet::write_ref_array_pre_work, shouldn't all new code go under the
> _heap->is_concurrent_mark_in_progress branch?

In-fact, we don't even have to check this flag, because it's already 
checked in the arraycopy_prologue-code generated by 
shenandoahBarrierSetAssembler_XXX.cpp.

*And* we can also avoid pointlessly checking ShenandoahSATBBarrier, if 
we do the check at codegen-time in the stub generator too.

I suppose this might be helpful if/because that code is often called 
with small/tiny arrays (actual workload).

> *) Indenting nit in shenandoahRuntime.cpp.
> 
>    52   shenandoah_assert_correct(NULL, orig);
>    53   // store the original value that was in the field reference
>    54 assert(ShenandoahThreadLocalData::satb_mark_queue(thread).is_active(), "Shouldn't be here
> otherwise");
>    55   ShenandoahThreadLocalData::satb_mark_queue(thread).enqueue_known_active(orig);

Fixed.

New webrev:
http://cr.openjdk.java.net/~rkennke/JDK-8221751/webrev.01/

hotspot_gc_shenandoah is still good.

Roman



More information about the hotspot-gc-dev mailing list