JDK-8213307 : G1 should clean up RMT with ClassUnloadingWithConcurrentMark
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: 12
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-11-02
  • Updated: 2018-11-15
  • Resolved: 2018-11-13
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 12
12 b20Fixed
Related Reports
Relates :  
Description
Found this when fixing a performance bug in Shenandoah, but I think G1 falls victim to the same thing. There is a block in G1ConcurrentMark::weak_refs_work:

  // Unload Klasses, String, Code Cache, etc.
  if (ClassUnloadingWithConcurrentMark) {
    GCTraceTime(Debug, gc, phases) debug("Class Unloading", _gc_timer_cm);
    bool purged_classes = SystemDictionary::do_unloading(_gc_timer_cm, false /* Defer cleaning */);
    _g1h->complete_cleaning(&g1_is_alive, purged_classes);
  } else {
   ...
  }

The salient part is "false /* Defer cleaning */". Shenandoah used to have the same "false" there, under the impression that ResolvedMethodTable cleanup would happen later with parallel cleaning. G1 would do that in _g1h->complete_cleaning(...) in the block above.

But, JDK-8206423 moved RMT cleanup from parallel cleanup, and it expects the trigger from SystemDictionary::do_unloading(), which is protected by that boolean flag, do_cleaning, which is false:

  if (do_cleaning) {
    GCTraceTime(Debug, gc, phases) t("ResolvedMethodTable", gc_timer);
    ResolvedMethodTable::trigger_cleanup();
  }

I think we should pass "do_cleaning = true" there now, to trigger RMT cleanup.

Possible reproducer:

diff -r d49c976b2a2a test/hotspot/jtreg/runtime/MemberName/MemberNameLeak.java
--- a/test/hotspot/jtreg/runtime/MemberName/MemberNameLeak.java Thu Nov 01 18:41:22 2018 +0100
+++ b/test/hotspot/jtreg/runtime/MemberName/MemberNameLeak.java Fri Nov 02 18:02:29 2018 +0100
@@ -75,10 +75,12 @@
         // Run this Leak class with logging
         ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
                                       "-Xlog:membername+table=trace",
                                       "-XX:+UnlockExperimentalVMOptions",
                                       "-XX:+UnlockDiagnosticVMOptions",
+                                      "-XX:+ClassUnloadingWithConcurrentMark",
+                                      "-XX:+ExplicitGCInvokesConcurrent",
                                       "-XX:+WhiteBoxAPI",
                                       "-Xbootclasspath/a:.",
                                       gc, Leak.class.getName());
         OutputAnalyzer output = new OutputAnalyzer(pb.start());
         output.shouldContain("ResolvedMethod entry added for MemberNameLeak$Leak.callMe()V");


$ make images run-test TEST=runtime/MemberName/MemberNameLeak.java

It used to time out with Shenandoah without "do_cleaning = true". It does get stuck with G1 too. Gets fixed with:

diff -r d49c976b2a2a src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp      Thu Nov 01 18:41:22 2018 +0100
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp      Fri Nov 02 18:04:28 2018 +0100
@@ -1651,11 +1651,11 @@
   }
 
   // Unload Klasses, String, Code Cache, etc.
   if (ClassUnloadingWithConcurrentMark) {
     GCTraceTime(Debug, gc, phases) debug("Class Unloading", _gc_timer_cm);
-    bool purged_classes = SystemDictionary::do_unloading(_gc_timer_cm, false /* Defer cleaning */);
+    bool purged_classes = SystemDictionary::do_unloading(_gc_timer_cm);
     _g1h->complete_cleaning(&g1_is_alive, purged_classes);
   } else {
     GCTraceTime(Debug, gc, phases) debug("Cleanup", _gc_timer_cm);
     // No need to clean string table as it is treated as strong roots when
     // class unloading is disabled.

Comments
Changing this to a bug as unloading RMT has been worked before JDK-8206423 during a concurrent mark cycle.
05-11-2018