8217778: StringTable cleanup miscalculates amount of dead objects
Reviewed-by: kbarrett
--- 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());
+ }
}
}