RFR: Make partial GC concurrent
Aleksey Shipilev
shade at redhat.com
Fri Oct 6 17:46:44 UTC 2017
On 10/06/2017 11:20 AM, Roman Kennke wrote:
> http://cr.openjdk.java.net/~rkennke/concpartial/webrev.01/
It looks like a great chunk of work!
First batch of comments:
*) It feels weird to have the accessor like SH::is_conc_partial_gc. For one, I thought it is a
"progress" accessor, that tripped me into believing this was the coding error. Then it became
obvious it selects the flavors of barriers. It seems to me that it's cleaner to continue our style
of global barriers flags. This means:
- Things like these would not be needed:
580 if (UseTLAB && ShenandoahAsmWB &&
581 ! ShenandoahHeap::heap()->is_conc_partial_gc()) {
...if we just disable ShenandoahAsmWB when is_conc_partial_gc() is off.
- Conditions like these would not be needed:
storeval_barrier:
if (_heap->is_conc_partial_gc()) {
...
} else {
...
}
...if we have separate ShenandoahStoreValReadBarrier and ShenandoahStoreValWriteBarrier. This
would also let us measure their respective overheads better
*) There is some weird interaction between ShenandoahKeepAliveBarrier and new concurrent partial
code. MacroAssembler::keep_alive_barrier would call shenandoah_write_barrier_pre, and that only get
executed when concurrent mark is active. Shouldn't it execute when concurrent partial is active too?
This gets weirder when ShenandoahBarrierSet::keep_alive_barrier executes WB when conc-partial is in
progress, but MacroAssembler::keep_alive_barrier does not.
*) It seems that !UseShenandoahMatrix paths in ShenandoahBarrierSet::write_ref_array,
ShenandoahBarrierSet::write_region_work are not needed, because no matrix -> no partial? Would need
to assert that though. But that keeps compiled code denser.
*) What's the purpose of ShenandoahConcMarkGC flag?
*) Yes, marking loops in shenandoahPartialGC.cpp should definitely be deduplicated.
*) ShenandoahPartialGC::concurrent_partial_collection() and final_partial_collection should
probably have a marker object with ShenandoahWorkerScope to set up appropriate number of concurrent
and parallel workers. This would need a new method in ShenandoahWorkerPolicy.
*) Checking for cancellation is not needed in ShenandoahPartialGC::final_partial_collection(),
because it is checked before?
521 if (ShenandoahVerify && ! _heap->cancelled_concgc()) {
Thanks,
-Aleksey
More information about the shenandoah-dev
mailing list