8171238: Unify cleanup code used in G1 Remark and Full GC marking

Reviewed-by: tschatzl, pliden

hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp

@@ -3531,9 +3531,11 @@ void G1CollectedHeap::print_termination_stats(uint worker_id,
                undo_waste * HeapWordSize / K);
 }
 
-class G1StringSymbolTableUnlinkTask : public AbstractGangTask {
+class G1StringAndSymbolCleaningTask : public AbstractGangTask {
 private:
   BoolObjectClosure* _is_alive;
+  G1StringDedupUnlinkOrOopsDoClosure _dedup_closure;
+
   int _initial_string_table_size;
   int _initial_symbol_table_size;
 
@@ -3545,12 +3547,16 @@ private:
   int _symbols_processed;
   int _symbols_removed;
 
+  bool _process_string_dedup;
+
 public:
-  G1StringSymbolTableUnlinkTask(BoolObjectClosure* is_alive, bool process_strings, bool process_symbols) :
+  G1StringAndSymbolCleaningTask(BoolObjectClosure* is_alive, bool process_strings, bool process_symbols, bool process_string_dedup) :
     AbstractGangTask("String/Symbol Unlinking"),
     _is_alive(is_alive),
+    _dedup_closure(is_alive, NULL, false),
     _process_strings(process_strings), _strings_processed(0), _strings_removed(0),
-    _process_symbols(process_symbols), _symbols_processed(0), _symbols_removed(0) {
+    _process_symbols(process_symbols), _symbols_processed(0), _symbols_removed(0),
+    _process_string_dedup(process_string_dedup) {
 
     _initial_string_table_size = StringTable::the_table()->table_size();
     _initial_symbol_table_size = SymbolTable::the_table()->table_size();
@@ -3562,7 +3568,7 @@ public:
     }
   }
 
-  ~G1StringSymbolTableUnlinkTask() {
+  ~G1StringAndSymbolCleaningTask() {
     guarantee(!_process_strings || StringTable::parallel_claimed_index() >= _initial_string_table_size,
               "claim value %d after unlink less than initial string table size %d",
               StringTable::parallel_claimed_index(), _initial_string_table_size);
@@ -3593,6 +3599,9 @@ public:
       Atomic::add(symbols_processed, &_symbols_processed);
       Atomic::add(symbols_removed, &_symbols_removed);
     }
+    if (_process_string_dedup) {
+      G1StringDedup::parallel_unlink(&_dedup_closure, worker_id);
+    }
   }
 
   size_t strings_processed() const { return (size_t)_strings_processed; }
@@ -3828,15 +3837,15 @@ public:
 // To minimize the remark pause times, the tasks below are done in parallel.
 class G1ParallelCleaningTask : public AbstractGangTask {
 private:
-  G1StringSymbolTableUnlinkTask _string_symbol_task;
+  G1StringAndSymbolCleaningTask _string_symbol_task;
   G1CodeCacheUnloadingTask      _code_cache_task;
   G1KlassCleaningTask           _klass_cleaning_task;
 
 public:
   // The constructor is run in the VMThread.
-  G1ParallelCleaningTask(BoolObjectClosure* is_alive, bool process_strings, bool process_symbols, uint num_workers, bool unloading_occurred) :
+  G1ParallelCleaningTask(BoolObjectClosure* is_alive, uint num_workers, bool unloading_occurred) :
       AbstractGangTask("Parallel Cleaning"),
-      _string_symbol_task(is_alive, process_strings, process_symbols),
+      _string_symbol_task(is_alive, true, true, G1StringDedup::is_enabled()),
       _code_cache_task(num_workers, is_alive, unloading_occurred),
       _klass_cleaning_task(is_alive) {
   }
@@ -3865,23 +3874,26 @@ public:
 };
 
 
-void G1CollectedHeap::parallel_cleaning(BoolObjectClosure* is_alive,
+void G1CollectedHeap::complete_cleaning(BoolObjectClosure* is_alive,
-                                        bool process_strings,
-                                        bool process_symbols,
                                         bool class_unloading_occurred) {
   uint n_workers = workers()->active_workers();
 
-  G1ParallelCleaningTask g1_unlink_task(is_alive, process_strings, process_symbols,
+  G1ParallelCleaningTask g1_unlink_task(is_alive, n_workers, class_unloading_occurred);
-                                        n_workers, class_unloading_occurred);
   workers()->run_task(&g1_unlink_task);
 }
 
-void G1CollectedHeap::unlink_string_and_symbol_table(BoolObjectClosure* is_alive,
+void G1CollectedHeap::partial_cleaning(BoolObjectClosure* is_alive,
-                                                     bool process_strings, bool process_symbols) {
+                                       bool process_strings,
-  { // Timing scope
+                                       bool process_symbols,
-    G1StringSymbolTableUnlinkTask g1_unlink_task(is_alive, process_strings, process_symbols);
+                                       bool process_string_dedup) {
-    workers()->run_task(&g1_unlink_task);
+  if (!process_strings && !process_symbols && !process_string_dedup) {
+    // Nothing to clean.
+    return;
   }
+
+  G1StringAndSymbolCleaningTask g1_unlink_task(is_alive, process_strings, process_symbols, process_string_dedup);
+  workers()->run_task(&g1_unlink_task);
+
 }
 
 class G1RedirtyLoggedCardsTask : public AbstractGangTask {

hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp

@@ -1403,12 +1403,16 @@ public:
   // after a full GC.
   void rebuild_strong_code_roots();
 
-  // Delete entries for dead interned string and clean up unreferenced symbols
+  // Partial cleaning used when class unloading is disabled.
-  // in symbol table, possibly in parallel.
+  // Let the caller choose what structures to clean out:
-  void unlink_string_and_symbol_table(BoolObjectClosure* is_alive, bool unlink_strings = true, bool unlink_symbols = true);
+  // - StringTable
+  // - SymbolTable
+  // - StringDeduplication structures
+  void partial_cleaning(BoolObjectClosure* is_alive, bool unlink_strings, bool unlink_symbols, bool unlink_string_dedup);
 
-  // Parallel phase of unloading/cleaning after G1 concurrent mark.
+  // Complete cleaning used when class unloading is enabled.
-  void parallel_cleaning(BoolObjectClosure* is_alive, bool process_strings, bool process_symbols, bool class_unloading_occurred);
+  // Cleans out all structures handled by partial_cleaning and also the CodeCache.
+  void complete_cleaning(BoolObjectClosure* is_alive, bool class_unloading_occurred);
 
   // Redirty logged cards in the refinement queue.
   void redirty_logged_cards();

hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp

@@ -1652,10 +1652,6 @@ void G1CMRefProcTaskExecutor::execute(EnqueueTask& enq_task) {
   _workers->run_task(&enq_task_proxy);
 }
 
-void G1ConcurrentMark::weakRefsWorkParallelPart(BoolObjectClosure* is_alive, bool purged_classes) {
-  G1CollectedHeap::heap()->parallel_cleaning(is_alive, true, true, purged_classes);
-}
-
 void G1ConcurrentMark::weakRefsWork(bool clear_all_soft_refs) {
   if (has_overflown()) {
     // Skip processing the discovered references if we have
@@ -1762,22 +1758,15 @@ void G1ConcurrentMark::weakRefsWork(bool clear_all_soft_refs) {
 
   // Unload Klasses, String, Symbols, Code Cache, etc.
   if (ClassUnloadingWithConcurrentMark) {
-    bool purged_classes;
+    GCTraceTime(Debug, gc, phases) debug("Class Unloading", _gc_timer_cm);
+    bool purged_classes = SystemDictionary::do_unloading(&g1_is_alive, false /* Defer klass cleaning */);
+    g1h->complete_cleaning(&g1_is_alive, purged_classes);
+  } else {
+    GCTraceTime(Debug, gc, phases) debug("Cleanup", _gc_timer_cm);
+    // No need to clean string table and symbol table as they are treated as strong roots when
+    // class unloading is disabled.
+    g1h->partial_cleaning(&g1_is_alive, false, false, G1StringDedup::is_enabled());
 
-    {
-      GCTraceTime(Debug, gc, phases) trace("System Dictionary Unloading", _gc_timer_cm);
-      purged_classes = SystemDictionary::do_unloading(&g1_is_alive, false /* Defer klass cleaning */);
-    }
-
-    {
-      GCTraceTime(Debug, gc, phases) trace("Parallel Unloading", _gc_timer_cm);
-      weakRefsWorkParallelPart(&g1_is_alive, purged_classes);
-    }
-  }
-
-  if (G1StringDedup::is_enabled()) {
-    GCTraceTime(Debug, gc, phases) trace("String Deduplication Unlink", _gc_timer_cm);
-    G1StringDedup::unlink(&g1_is_alive);
   }
 }
 

hotspot/src/share/vm/gc/g1/g1MarkSweep.cpp

@@ -167,22 +167,10 @@ void G1MarkSweep::mark_sweep_phase1(bool& marked_for_unloading,
     // Unload classes and purge the SystemDictionary.
     bool purged_class = SystemDictionary::do_unloading(&GenMarkSweep::is_alive);
 
-    // Unload nmethods.
+    g1h->complete_cleaning(&GenMarkSweep::is_alive, purged_class);
-    CodeCache::do_unloading(&GenMarkSweep::is_alive, purged_class);
+  } else {
-
+    GCTraceTime(Debug, gc, phases) trace("Cleanup", gc_timer());
-    // Prune dead klasses from subklass/sibling/implementor lists.
+    g1h->partial_cleaning(&GenMarkSweep::is_alive, true, true, G1StringDedup::is_enabled());
-    Klass::clean_weak_klass_links(&GenMarkSweep::is_alive);
-  }
-
-  {
-    GCTraceTime(Debug, gc, phases) trace("Scrub String and Symbol Tables", gc_timer());
-    // Delete entries for dead interned string and clean up unreferenced symbols in symbol table.
-    g1h->unlink_string_and_symbol_table(&GenMarkSweep::is_alive);
-  }
-
-  if (G1StringDedup::is_enabled()) {
-    GCTraceTime(Debug, gc, phases) trace("String Deduplication Unlink", gc_timer());
-    G1StringDedup::unlink(&GenMarkSweep::is_alive);
   }
 
   if (VerifyDuringGC) {

hotspot/src/share/vm/gc/g1/g1StringDedup.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -109,11 +109,10 @@ void G1StringDedup::oops_do(OopClosure* keep_alive) {
   unlink_or_oops_do(NULL, keep_alive, true /* allow_resize_and_rehash */);
 }
 
-void G1StringDedup::unlink(BoolObjectClosure* is_alive) {
+void G1StringDedup::parallel_unlink(G1StringDedupUnlinkOrOopsDoClosure* unlink, uint worker_id) {
   assert(is_enabled(), "String deduplication not enabled");
-  // Don't allow a potential resize or rehash during unlink, as the unlink
+  G1StringDedupQueue::unlink_or_oops_do(unlink);
-  // operation itself might remove enough entries to invalidate such a decision.
+  G1StringDedupTable::unlink_or_oops_do(unlink, worker_id);
-  unlink_or_oops_do(is_alive, NULL, false /* allow_resize_and_rehash */);
 }
 
 //

hotspot/src/share/vm/gc/g1/g1StringDedup.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -90,6 +90,7 @@ class BoolObjectClosure;
 class ThreadClosure;
 class outputStream;
 class G1StringDedupTable;
+class G1StringDedupUnlinkOrOopsDoClosure;
 class G1GCPhaseTimes;
 
 //
@@ -129,7 +130,7 @@ public:
                                       unsigned int queue, oop java_string);
 
   static void oops_do(OopClosure* keep_alive);
-  static void unlink(BoolObjectClosure* is_alive);
+  static void parallel_unlink(G1StringDedupUnlinkOrOopsDoClosure* unlink, uint worker_id);
   static void unlink_or_oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive,
                                 bool allow_resize_and_rehash, G1GCPhaseTimes* phase_times = NULL);