RFR: Workaround degen-traversal bugs with evac-assists enabled

[Aleksey Shipilev]

I have had a head-scratcher bug during backports testing, which boils down to bad degen-traversal
and evac-assists interaction. Let's turn that off for traversal, until we come up with the proper fix:

diff -r dbd712ba015c src/hotspot/share/gc/shenandoah/heuristics/shenandoahTraversalHeuristics.cpp
--- a/src/hotspot/share/gc/shenandoah/heuristics/shenandoahTraversalHeuristics.cpp      Mon Aug 20
12:32:27 2018 -0400
+++ b/src/hotspot/share/gc/shenandoah/heuristics/shenandoahTraversalHeuristics.cpp      Tue Aug 21
11:14:05 2018 +0200
@@ -47,10 +47,32 @@
   // Adjust class unloading settings only if globally enabled.
   if (ClassUnloadingWithConcurrentMark) {
     SHENANDOAH_ERGO_OVERRIDE_DEFAULT(ShenandoahUnloadClassesFrequency, 1);
   }

+  // Workaround the bug in degen-traversal that evac assists expose.
+  //
+  // During traversal cycle, WB can evacuate something from region R1 (CS) to R2 (R).
+  // Then degen-traversal happens, drops the cset, and finishes up the fixups.
+  // Then next cycle happens to put both R1 and R2 into CS, and then R2 evacuates to R3.
+  // It creates the double forwarding for that object: R1 (CS) -> R2 (CS) -> R3 (R).
+  //
+  // It is likely at that point that no references to R1 object are left after the degen,
+  // so this double forwarding is not exposed. Unless we have evac assists, that touch
+  // the adjacent objects while evacuating live objects from R1, step on "bad" R1 copy,
+  // and fail the internal asserts when getting oop sizes to walk the heap, or touching
+  // its fwdptrs. The same thing would probably happen if we do size-based iteration
+  // somewhere else.
+  //
+  // AllocHumongousFragment test exposes it nicely, always running into degens.
+  //
+  // TODO: Fix this properly
+  // There are two alternatives: fix it in degen so that it never leaves double forwarding,
+  // or make sure we only use raw accessors in evac assist path when getting oop_size,
+  // including all exotic shapes like instanceMirrorKlass, and touching fwdptrs. The second
+  // option is partly done in jdk12, but not in earlier jdks.
+  FLAG_SET_DEFAULT(ShenandoahEvacAssist, 0);
 }

 bool ShenandoahTraversalHeuristics::should_start_normal_gc() const {
   return false;
 }
diff -r dbd712ba015c src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp        Mon Aug 20 12:32:27 2018 -0400
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp        Tue Aug 21 11:14:05 2018 +0200
@@ -1888,10 +1888,14 @@
     // but enters it at different points, depending on which concurrent phase had
     // degenerated.

     case _degenerated_traversal:
       {
+        // Drop the collection set. Note: this leaves some already forwarded objects
+        // behind, which may be problematic, see comments for ShenandoahEvacAssist
+        // workarounds in ShenandoahTraversalHeuristics.
+
         ShenandoahHeapLocker locker(lock());
         collection_set()->clear_current_index();
         for (size_t i = 0; i < collection_set()->count(); i++) {
           ShenandoahHeapRegion* r = collection_set()->next();
           r->make_regular_bypass();


Testing: tier3_gc_shenandoah

Thanks,
-Aleksey