RFR: 8242054: Shenandoah: New incremental-update mode

Aleksey Shipilev shade at redhat.com
Mon Apr 6 12:28:51 UTC 2020


On 4/6/20 2:21 PM, Roman Kennke wrote:
> 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/

*) shenandoahBarrierSetAssembler_aarch64.cpp: flag check should be first?

  62       if (dest_uninitialized && !ShenandoahStoreValEnqueueBarrier) {

*) I wonder if there is a test that checks ShenandoahSATBBarrier and
ShenandoahStoreValEnqueueBarrier are not enabled at the same time. If not, I can add it later.

*) I wonder ShenandoahBarrierSet::clone_barrier should actually check for ShenandoahHeap::UPDATEREFS
on L110, like ShenandoahBarrierSet::arraycopy_barrier started doing it recently?
  http://hg.openjdk.java.net/jdk/jdk/rev/0c8345a2ad50#l4.36

 105   int gc_state = _heap->gc_state();
 106   if ((gc_state & ShenandoahHeap::MARKING) != 0) {
 107     clone_marking(obj);
 108   } else if ((gc_state & ShenandoahHeap::EVACUATION) != 0) {
 109     clone_evacuation(obj);
 110   } else {
 111     clone_update(obj);
 112   }

*) In ShenandoahSATBAndRemarkCodeRootsThreadsClosure, double space in "else  if" here:
 202         } else  if (_code_cl != NULL) {

*) In TestObjItrWithHeapDump.java, the indent is wrong:
  60              {{"normal"},    {"adaptive", "compact", "static", "aggressive"}},
  61              {{"iu"}, {"adaptive", "aggressive"}},
  62              {{"passive"},   {"passive"}}

*) Same thing in TestClassLoaderLeak.java:

 128         String[][][] modeHeuristics = new String[][][] {
 129              {{"normal"},    {"adaptive", "compact", "static", "aggressive"}},
 130              {{"iu"}, {"adaptive", "aggressive"}},
 131              {{"passive"},   {"passive"}}
 132         };

*) In TestWrongArrayMember.java, the indent is wrong:

  30  * @run main/othervm -Xmx128m -XX:+UnlockExperimentalVMOptions -XX:+UseShenandoahGC
                    TestWrongArrayMember
  31  * @run main/othervm -Xmx128m -XX:+UnlockExperimentalVMOptions -XX:+UseShenandoahGC
-XX:ShenandoahGCMode=iu TestWrongArrayMember

*) TestHeuristicsUnlock.java change actually raises a question: should this mode be experimental?
  52         testWith("-XX:ShenandoahGCMode=iu", Mode.PRODUCT);


Otherwise looks fine!


-- 
Thanks,
-Aleksey



More information about the shenandoah-dev mailing list