8204613: StringTable: Calculates wrong number of uncleaned items.
Reviewed-by: pliden, coleenp
--- a/src/hotspot/share/classfile/stringTable.cpp Thu Jun 14 09:48:04 2018 +0800
+++ b/src/hotspot/share/classfile/stringTable.cpp Thu Jun 14 07:26:27 2018 +0200
@@ -198,17 +198,16 @@
return Atomic::add((size_t)1, &(the_table()->_items));
}
-size_t StringTable::items_to_clean(size_t ncl) {
- size_t total = Atomic::add((size_t)ncl, &(the_table()->_uncleaned_items));
+size_t StringTable::add_items_to_clean(size_t ndead) {
+ size_t total = Atomic::add((size_t)ndead, &(the_table()->_uncleaned_items));
log_trace(stringtable)(
"Uncleaned items:" SIZE_FORMAT " added: " SIZE_FORMAT " total:" SIZE_FORMAT,
- the_table()->_uncleaned_items, ncl, total);
+ the_table()->_uncleaned_items, ndead, total);
return total;
}
void StringTable::item_removed() {
Atomic::add((size_t)-1, &(the_table()->_items));
- Atomic::add((size_t)-1, &(the_table()->_uncleaned_items));
}
double StringTable::get_load_factor() {
@@ -405,8 +404,11 @@
StringTable::the_table()->_weak_handles->weak_oops_do(&stiac, tmp);
- StringTable::the_table()->items_to_clean(stiac._count);
+ // This is the serial case without ParState.
+ // Just set the correct number and check for a cleaning phase.
+ the_table()->_uncleaned_items = stiac._count;
StringTable::the_table()->check_concurrent_work();
+
if (processed != NULL) {
*processed = (int) stiac._count_total;
}
@@ -430,8 +432,9 @@
_par_state_string->weak_oops_do(&stiac, &dnc);
- StringTable::the_table()->items_to_clean(stiac._count);
- StringTable::the_table()->check_concurrent_work();
+ // Accumulate the dead strings.
+ the_table()->add_items_to_clean(stiac._count);
+
*processed = (int) stiac._count_total;
*removed = (int) stiac._count;
}
@@ -467,10 +470,8 @@
}
struct StringTableDoDelete : StackObj {
- long _count;
- StringTableDoDelete() : _count(0) {}
void operator()(WeakHandle<vm_string_table_data>* val) {
- ++_count;
+ /* do nothing */
}
};
@@ -524,6 +525,7 @@
if (_has_work) {
return;
}
+
double load_factor = StringTable::get_load_factor();
double dead_factor = StringTable::get_dead_factor();
// We should clean/resize if we have more dead than alive,
--- a/src/hotspot/share/classfile/stringTable.hpp Thu Jun 14 09:48:04 2018 +0800
+++ b/src/hotspot/share/classfile/stringTable.hpp Thu Jun 14 07:26:27 2018 +0200
@@ -83,7 +83,7 @@
static uintx item_added();
static void item_removed();
- static size_t items_to_clean(size_t ncl);
+ size_t add_items_to_clean(size_t ndead);
StringTable();
@@ -113,6 +113,23 @@
static bool has_work() { return the_table()->_has_work; }
// GC support
+
+ // Must be called before a parallel walk where strings might die.
+ static void reset_dead_counter() {
+ the_table()->_uncleaned_items = 0;
+ }
+ // After the parallel walk this method must be called to trigger
+ // cleaning. Note it might trigger a resize instead.
+ static void finish_dead_counter() {
+ the_table()->check_concurrent_work();
+ }
+
+ // If GC uses ParState directly it should add the number of cleared
+ // strings to this method.
+ static void inc_dead_counter(size_t ndead) {
+ the_table()->add_items_to_clean(ndead);
+ }
+
// Delete pointers to otherwise-unreachable objects.
static void unlink(BoolObjectClosure* cl) {
unlink_or_oops_do(cl);
@@ -150,9 +167,9 @@
oop lookup_shared(jchar* name, int len, unsigned int hash) NOT_CDS_JAVA_HEAP_RETURN_(NULL);
static void copy_shared_string_table(CompactStringTableWriter* ch_table) NOT_CDS_JAVA_HEAP_RETURN;
public:
- static oop create_archived_string(oop s, Thread* THREAD);
+ static oop create_archived_string(oop s, Thread* THREAD) NOT_CDS_JAVA_HEAP_RETURN_(NULL);
static void set_shared_string_mapped() { _shared_string_mapped = true; }
- static bool shared_string_mapped() { return _shared_string_mapped; }
+ static bool shared_string_mapped() { return _shared_string_mapped; }
static void shared_oops_do(OopClosure* f) NOT_CDS_JAVA_HEAP_RETURN;
static void write_to_archive() NOT_CDS_JAVA_HEAP_RETURN;
static void serialize(SerializeClosure* soc) NOT_CDS_JAVA_HEAP_RETURN;
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Thu Jun 14 09:48:04 2018 +0800
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Thu Jun 14 07:26:27 2018 +0200
@@ -3262,6 +3262,9 @@
if (process_symbols) {
SymbolTable::clear_parallel_claimed_index();
}
+ if (process_strings) {
+ StringTable::reset_dead_counter();
+ }
}
~G1StringAndSymbolCleaningTask() {
@@ -3275,6 +3278,9 @@
"symbols: " SIZE_FORMAT " processed, " SIZE_FORMAT " removed",
strings_processed(), strings_removed(),
symbols_processed(), symbols_removed());
+ if (_process_strings) {
+ StringTable::finish_dead_counter();
+ }
}
void work(uint worker_id) {
--- a/src/hotspot/share/gc/z/zRootsIterator.cpp Thu Jun 14 09:48:04 2018 +0800
+++ b/src/hotspot/share/gc/z/zRootsIterator.cpp Thu Jun 14 07:26:27 2018 +0200
@@ -307,10 +307,12 @@
assert(SafepointSynchronize::is_at_safepoint(), "Should be at safepoint");
ZStatTimer timer(ZSubPhasePauseWeakRootsSetup);
SymbolTable::clear_parallel_claimed_index();
+ StringTable::reset_dead_counter();
}
ZWeakRootsIterator::~ZWeakRootsIterator() {
ZStatTimer timer(ZSubPhasePauseWeakRootsTeardown);
+ StringTable::finish_dead_counter();
}
void ZWeakRootsIterator::do_vm_weak_handles(BoolObjectClosure* is_alive, OopClosure* cl) {
@@ -341,9 +343,34 @@
SymbolTable::possibly_parallel_unlink(&dummy, &dummy);
}
+class ZStringTableDeadCounterBoolObjectClosure : public BoolObjectClosure {
+private:
+ BoolObjectClosure* const _cl;
+ size_t _ndead;
+
+public:
+ ZStringTableDeadCounterBoolObjectClosure(BoolObjectClosure* cl) :
+ _cl(cl),
+ _ndead(0) {}
+
+ ~ZStringTableDeadCounterBoolObjectClosure() {
+ StringTable::inc_dead_counter(_ndead);
+ }
+
+ virtual bool do_object_b(oop obj) {
+ if (_cl->do_object_b(obj)) {
+ return true;
+ }
+
+ _ndead++;
+ return false;
+ }
+};
+
void ZWeakRootsIterator::do_string_table(BoolObjectClosure* is_alive, OopClosure* cl) {
ZStatTimer timer(ZSubPhasePauseWeakRootsStringTable);
- _string_table_iter.weak_oops_do(is_alive, cl);
+ ZStringTableDeadCounterBoolObjectClosure counter_is_alive(is_alive);
+ _string_table_iter.weak_oops_do(&counter_is_alive, cl);
}
void ZWeakRootsIterator::weak_oops_do(BoolObjectClosure* is_alive, OopClosure* cl) {
@@ -377,7 +404,13 @@
_string_table_iter(StringTable::weak_storage()),
_vm_weak_handles(this),
_jni_weak_handles(this),
- _string_table(this) {}
+ _string_table(this) {
+ StringTable::reset_dead_counter();
+}
+
+ZConcurrentWeakRootsIterator::~ZConcurrentWeakRootsIterator() {
+ StringTable::finish_dead_counter();
+}
void ZConcurrentWeakRootsIterator::do_vm_weak_handles(OopClosure* cl) {
ZStatTimer timer(ZSubPhaseConcurrentWeakRootsVMWeakHandles);
@@ -389,9 +422,36 @@
_jni_weak_handles_iter.oops_do(cl);
}
+class ZStringTableDeadCounterOopClosure : public OopClosure {
+private:
+ OopClosure* const _cl;
+ size_t _ndead;
+
+public:
+ ZStringTableDeadCounterOopClosure(OopClosure* cl) :
+ _cl(cl),
+ _ndead(0) {}
+
+ ~ZStringTableDeadCounterOopClosure() {
+ StringTable::inc_dead_counter(_ndead);
+ }
+
+ virtual void do_oop(oop* p) {
+ _cl->do_oop(p);
+ if (*p == NULL) {
+ _ndead++;
+ }
+ }
+
+ virtual void do_oop(narrowOop* p) {
+ ShouldNotReachHere();
+ }
+};
+
void ZConcurrentWeakRootsIterator::do_string_table(OopClosure* cl) {
ZStatTimer timer(ZSubPhaseConcurrentWeakRootsStringTable);
- _string_table_iter.oops_do(cl);
+ ZStringTableDeadCounterOopClosure counter_cl(cl);
+ _string_table_iter.oops_do(&counter_cl);
}
void ZConcurrentWeakRootsIterator::oops_do(OopClosure* cl) {
--- a/src/hotspot/share/gc/z/zRootsIterator.hpp Thu Jun 14 09:48:04 2018 +0800
+++ b/src/hotspot/share/gc/z/zRootsIterator.hpp Thu Jun 14 07:26:27 2018 +0200
@@ -164,6 +164,7 @@
public:
ZConcurrentWeakRootsIterator();
+ ~ZConcurrentWeakRootsIterator();
void oops_do(OopClosure* cl);
};