RFR: Traveral GC heuristics
Aleksey Shipilev
shade at redhat.com
Fri Jan 19 10:24:40 UTC 2018
On 01/17/2018 10:58 PM, Roman Kennke wrote:
> Yes, maybe. But for the start, I did not want it to interfere with existing code if I can avoid
> it. For this reason, this looks like a copy+paste job from conc-mark and partial for some parts.
Okay, but please plan to common these things right away. We cannot have two copy-pasted 1000+ LOC
blocks and hope for the best ;)
> Thanks for reviewing and spotting all the issues. I could not really make a diff webrev, because I
> first had to pull -u your latest work, and this messed up my differential webrev... sorry. Only full
> webrev now:
>
> http://cr.openjdk.java.net/~rkennke/traversal/webrev.03/
Sorry to be a PITA about this, but the change is quite large, and I think we want to be more
forward-looking to backports and stability.
Another sweep through the code:
*) GCCause::to_string misses the to_string case for _shenandoah_traversal_gc?
*) So, wait. SBS::nterpreter_write_barrier_impl caller-saves registers when they do not equal to
dst. New code in SBS::interpreter_storeval_barrier just does it unconditionally. Is WB too cautious,
or SVB is too lax about this?
*) I think with minimal changes, we can make ShenandoahStoreValEnqueueBarrier exclusive, which will
make testing much easier (encoding this in TestSelectiveBarriers would be trivial). E.g. say:
if (UseShenandoahGC) {
if (ShenandoahStoreValWriteBarrier || ShenandoahStoreValEnqueueBarrier) {
// perform WB
}
if (ShenandoahStoreValEnqueueBarrier) {
// enqueue
}
if (ShenandoahStoreValReadBarrier) {
// RB
}
}
*) Minor nit: please indent second arguments like this:
FLAG_SET_DEFAULT(UseShenandoahMatrix, false);
FLAG_SET_DEFAULT(ShenandoahSATBBarrier, false);
FLAG_SET_DEFAULT(ShenandoahConditionalSATBBarrier, false);
FLAG_SET_DEFAULT(ShenandoahStoreValReadBarrier, false);
FLAG_SET_DEFAULT(ShenandoahStoreValWriteBarrier, true);
FLAG_SET_DEFAULT(ShenandoahStoreValEnqueueBarrier, true);
FLAG_SET_DEFAULT(ShenandoahKeepAliveBarrier, false);
FLAG_SET_DEFAULT(ShenandoahAsmWB, true);
FLAG_SET_DEFAULT(ShenandoahBarriersForConst, true);
FLAG_SET_DEFAULT(ShenandoahWBWithMemBar, false);
FLAG_SET_DEFAULT(ShenandoahWriteBarrierRB, false);
*) shenandoahOopClosures.hpp, indenting is a bit off here:
240 _thread(Thread::current()), _queue(q) {}
...
273 virtual bool do_metadata() { return true; }
*) I wonder if we want to pull out ShenandoahWBWithMemBar changes into a separate changeset? This
looks potentially backportable, and usable outside of Traversal GC.
Thanks,
-Aleksey
More information about the shenandoah-dev
mailing list