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