8217778: StringTable cleanup miscalculates amount of dead objects
authortschatzl
Fri, 08 Feb 2019 12:55:20 +0100
changeset 53702 50a5d0353570
parent 53701 e57bcfd7bf79
child 53703 24341625d8f2
8217778: StringTable cleanup miscalculates amount of dead objects Reviewed-by: kbarrett
src/hotspot/share/gc/shared/weakProcessor.cpp
src/hotspot/share/gc/shared/weakProcessor.inline.hpp
--- a/src/hotspot/share/gc/shared/weakProcessor.cpp	Fri Feb 08 13:07:44 2019 +0000
+++ b/src/hotspot/share/gc/shared/weakProcessor.cpp	Fri Feb 08 12:55:20 2019 +0100
@@ -35,19 +35,23 @@
 #include "utilities/macros.hpp"
 
 void WeakProcessor::weak_oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive) {
-  StringTable::reset_dead_counter();
-  CountingIsAliveClosure<BoolObjectClosure> cl(is_alive);
   FOR_EACH_WEAK_PROCESSOR_PHASE(phase) {
     if (WeakProcessorPhases::is_serial(phase)) {
-      WeakProcessorPhases::processor(phase)(&cl, keep_alive);
+      WeakProcessorPhases::processor(phase)(is_alive, keep_alive);
     } else {
-      WeakProcessorPhases::oop_storage(phase)->weak_oops_do(&cl, keep_alive);
-    }
-    if (WeakProcessorPhases::is_stringtable(phase)) {
-      StringTable::inc_dead_counter(cl.num_dead());
+      if (WeakProcessorPhases::is_stringtable(phase)) {
+        StringTable::reset_dead_counter();
+
+        CountingSkippedIsAliveClosure<BoolObjectClosure, OopClosure> cl(is_alive, keep_alive);
+        WeakProcessorPhases::oop_storage(phase)->oops_do(&cl);
+
+        StringTable::inc_dead_counter(cl.num_dead() + cl.num_skipped());
+        StringTable::finish_dead_counter();
+      } else {
+        WeakProcessorPhases::oop_storage(phase)->weak_oops_do(is_alive, keep_alive);
+      }
     }
   }
-  StringTable::finish_dead_counter();
 }
 
 void WeakProcessor::oops_do(OopClosure* closure) {
--- a/src/hotspot/share/gc/shared/weakProcessor.inline.hpp	Fri Feb 08 13:07:44 2019 +0000
+++ b/src/hotspot/share/gc/shared/weakProcessor.inline.hpp	Fri Feb 08 12:55:20 2019 +0100
@@ -37,15 +37,15 @@
 class BoolObjectClosure;
 class OopClosure;
 
-template<typename T>
+template<typename IsAlive>
 class CountingIsAliveClosure : public BoolObjectClosure {
-  T* _inner;
+  IsAlive* _inner;
 
   size_t _num_dead;
   size_t _num_total;
 
 public:
-  CountingIsAliveClosure(T* cl) : _inner(cl), _num_dead(0), _num_total(0) { }
+  CountingIsAliveClosure(IsAlive* cl) : _inner(cl), _num_dead(0), _num_total(0) { }
 
   virtual bool do_object_b(oop obj) {
     bool result = _inner->do_object_b(obj);
@@ -58,6 +58,33 @@
   size_t num_total() const { return _num_total; }
 };
 
+template <typename IsAlive, typename KeepAlive>
+class CountingSkippedIsAliveClosure : public Closure {
+  CountingIsAliveClosure<IsAlive> _counting_is_alive;
+  KeepAlive* _keep_alive;
+
+  size_t _num_skipped;
+
+public:
+  CountingSkippedIsAliveClosure(IsAlive* is_alive, KeepAlive* keep_alive) :
+    _counting_is_alive(is_alive), _keep_alive(keep_alive), _num_skipped(0) { }
+
+  void do_oop(oop* p) {
+    oop obj = *p;
+    if (obj == NULL) {
+      _num_skipped++;
+    } else if (_counting_is_alive.do_object_b(obj)) {
+      _keep_alive->do_oop(p);
+    } else {
+      *p = NULL;
+    }
+  }
+
+  size_t num_dead() const { return _counting_is_alive.num_dead(); }
+  size_t num_skipped() const { return _num_skipped; }
+  size_t num_total() const { return _counting_is_alive.num_total() + num_skipped(); }
+};
+
 template<typename IsAlive, typename KeepAlive>
 void WeakProcessor::Task::work(uint worker_id,
                                IsAlive* is_alive,
@@ -67,8 +94,8 @@
          worker_id, _nworkers);
 
   FOR_EACH_WEAK_PROCESSOR_PHASE(phase) {
-    CountingIsAliveClosure<IsAlive> cl(is_alive);
     if (WeakProcessorPhases::is_serial(phase)) {
+      CountingIsAliveClosure<IsAlive> cl(is_alive);
       uint serial_index = WeakProcessorPhases::serial_index(phase);
       if (_serial_phases_done.try_claim_task(serial_index)) {
         WeakProcessorPhaseTimeTracker pt(_phase_times, phase);
@@ -78,15 +105,16 @@
         }
       }
     } else {
+      CountingSkippedIsAliveClosure<IsAlive, KeepAlive> cl(is_alive, keep_alive);
       WeakProcessorPhaseTimeTracker pt(_phase_times, phase, worker_id);
       uint storage_index = WeakProcessorPhases::oop_storage_index(phase);
-      _storage_states[storage_index].weak_oops_do(&cl, keep_alive);
+      _storage_states[storage_index].oops_do(&cl);
       if (_phase_times != NULL) {
         _phase_times->record_worker_items(worker_id, phase, cl.num_dead(), cl.num_total());
       }
-    }
-    if (WeakProcessorPhases::is_stringtable(phase)) {
-      StringTable::inc_dead_counter(cl.num_dead());
+      if (WeakProcessorPhases::is_stringtable(phase)) {
+        StringTable::inc_dead_counter(cl.num_dead() + cl.num_skipped());
+      }
     }
   }