stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8180048: Interned string and symbol table leak memory during parallel unlinking

Browse Source 
Make appending found dead BasicHashtableEntrys to the free list atomic.

Reviewed-by: ehelin, shade, coleenp
This commit is contained in:
Thomas Schatzl 2017-05-15 12:20:15 +02:00
parent 8e28d5772d
commit c775f6f58b
7 changed files with 97 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
hotspot/src/share/vm/classfile/stringTable.cpp
View File

@ -314,7 +314,11 @@ oop StringTable::intern(const char* utf8_string, TRAPS) {
}
void StringTable::unlink_or_oops_do(BoolObjectClosure* is_alive, OopClosure* f, int* processed, int* removed) {
buckets_unlink_or_oops_do(is_alive, f, 0, the_table()->table_size(), processed, removed);
BucketUnlinkContext context;
buckets_unlink_or_oops_do(is_alive, f, 0, the_table()->table_size(), &context);
_the_table->bulk_free_entries(&context);
*processed = context._num_processed;
*removed = context._num_removed;
}
void StringTable::possibly_parallel_unlink_or_oops_do(BoolObjectClosure* is_alive, OopClosure* f, int* processed, int* removed) {
@ -323,6 +327,7 @@ void StringTable::possibly_parallel_unlink_or_oops_do(BoolObjectClosure* is_aliv
assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
const int limit = the_table()->table_size();
BucketUnlinkContext context;
for (;;) {
// Grab next set of buckets to scan
int start_idx = Atomic::add(ClaimChunkSize, &_parallel_claimed_idx) - ClaimChunkSize;
@ -332,8 +337,11 @@ void StringTable::possibly_parallel_unlink_or_oops_do(BoolObjectClosure* is_aliv
}
int end_idx = MIN2(limit, start_idx + ClaimChunkSize);
buckets_unlink_or_oops_do(is_alive, f, start_idx, end_idx, processed, removed);
buckets_unlink_or_oops_do(is_alive, f, start_idx, end_idx, &context);
}
_the_table->bulk_free_entries(&context);
*processed = context._num_processed;
*removed = context._num_removed;
}
void StringTable::buckets_oops_do(OopClosure* f, int start_idx, int end_idx) {
@ -359,7 +367,7 @@ void StringTable::buckets_oops_do(OopClosure* f, int start_idx, int end_idx) {
}
}
void StringTable::buckets_unlink_or_oops_do(BoolObjectClosure* is_alive, OopClosure* f, int start_idx, int end_idx, int* processed, int* removed) {
void StringTable::buckets_unlink_or_oops_do(BoolObjectClosure* is_alive, OopClosure* f, int start_idx, int end_idx, BucketUnlinkContext* context) {
const int limit = the_table()->table_size();
assert(0 <= start_idx && start_idx <= limit,
@ -383,10 +391,9 @@ void StringTable::buckets_unlink_or_oops_do(BoolObjectClosure* is_alive, OopClos
p = entry->next_addr();
} else {
*p = entry->next();
the_table()->free_entry(entry);
(*removed)++;
context->free_entry(entry);
}
(*processed)++;
context->_num_processed++;
entry = *p;
}
}

8
hotspot/src/share/vm/classfile/stringTable.hpp
View File

@ -61,9 +61,13 @@ private:
// Apply the give oop closure to the entries to the buckets
// in the range [start_idx, end_idx).
static void buckets_oops_do(OopClosure* f, int start_idx, int end_idx);
typedef StringTable::BucketUnlinkContext BucketUnlinkContext;
// Unlink or apply the give oop closure to the entries to the buckets
// in the range [start_idx, end_idx).
static void buckets_unlink_or_oops_do(BoolObjectClosure* is_alive, OopClosure* f, int start_idx, int end_idx, int* processed, int* removed);
// in the range [start_idx, end_idx). Unlinked bucket entries are collected in the given
// context to be freed later.
// This allows multiple threads to work on the table at once.
static void buckets_unlink_or_oops_do(BoolObjectClosure* is_alive, OopClosure* f, int start_idx, int end_idx, BucketUnlinkContext* context);
// Hashing algorithm, used as the hash value used by the
// StringTable for bucket selection and comparison (stored in the

35
hotspot/src/share/vm/classfile/symbolTable.cpp
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 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
@ -98,7 +98,7 @@ int SymbolTable::_symbols_removed = 0;
int SymbolTable::_symbols_counted = 0;
volatile int SymbolTable::_parallel_claimed_idx = 0;
void SymbolTable::buckets_unlink(int start_idx, int end_idx, int* processed, int* removed) {
void SymbolTable::buckets_unlink(int start_idx, int end_idx, BucketUnlinkContext* context) {
for (int i = start_idx; i < end_idx; ++i) {
HashtableEntry<Symbol*, mtSymbol>** p = the_table()->bucket_addr(i);
HashtableEntry<Symbol*, mtSymbol>* entry = the_table()->bucket(i);
@ -111,15 +111,14 @@ void SymbolTable::buckets_unlink(int start_idx, int end_idx, int* processed, int
break;
}
Symbol* s = entry->literal();
(*processed)++;
context->_num_processed++;
assert(s != NULL, "just checking");
// If reference count is zero, remove.
if (s->refcount() == 0) {
assert(!entry->is_shared(), "shared entries should be kept live");
delete s;
(*removed)++;
*p = entry->next();
the_table()->free_entry(entry);
context->free_entry(entry);
} else {
p = entry->next_addr();
}
@ -132,17 +131,20 @@ void SymbolTable::buckets_unlink(int start_idx, int end_idx, int* processed, int
// Remove unreferenced symbols from the symbol table
// This is done late during GC.
void SymbolTable::unlink(int* processed, int* removed) {
size_t memory_total = 0;
buckets_unlink(0, the_table()->table_size(), processed, removed);
_symbols_removed += *removed;
_symbols_counted += *processed;
BucketUnlinkContext context;
buckets_unlink(0, the_table()->table_size(), &context);
_the_table->bulk_free_entries(&context);
*processed = context._num_processed;
*removed = context._num_removed;
_symbols_removed = context._num_removed;
_symbols_counted = context._num_processed;
}
void SymbolTable::possibly_parallel_unlink(int* processed, int* removed) {
const int limit = the_table()->table_size();
size_t memory_total = 0;
BucketUnlinkContext context;
for (;;) {
// Grab next set of buckets to scan
int start_idx = Atomic::add(ClaimChunkSize, &_parallel_claimed_idx) - ClaimChunkSize;
@ -152,10 +154,15 @@ void SymbolTable::possibly_parallel_unlink(int* processed, int* removed) {
}
int end_idx = MIN2(limit, start_idx + ClaimChunkSize);
buckets_unlink(start_idx, end_idx, processed, removed);
buckets_unlink(start_idx, end_idx, &context);
}
Atomic::add(*processed, &_symbols_counted);
Atomic::add(*removed, &_symbols_removed);
_the_table->bulk_free_entries(&context);
*processed = context._num_processed;
*removed = context._num_removed;
Atomic::add(context._num_processed, &_symbols_counted);
Atomic::add(context._num_removed, &_symbols_removed);
}
// Create a new table and using alternate hash code, populate the new table

9
hotspot/src/share/vm/classfile/symbolTable.hpp
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 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
@ -154,8 +154,11 @@ private:
static volatile int _parallel_claimed_idx;
// Release any dead symbols
static void buckets_unlink(int start_idx, int end_idx, int* processed, int* removed);
typedef SymbolTable::BucketUnlinkContext BucketUnlinkContext;
// Release any dead symbols. Unlinked bucket entries are collected in the given
// context to be freed later.
// This allows multiple threads to work on the table at once.
static void buckets_unlink(int start_idx, int end_idx, BucketUnlinkContext* context);
public:
enum {
symbol_alloc_batch_size = 8,

2
hotspot/src/share/vm/runtime/vmStructs.cpp
View File

@ -673,7 +673,7 @@ typedef CompactHashtable<Symbol*, char> SymbolCompactHashTable;
\
nonstatic_field(BasicHashtable<mtInternal>, _table_size, int) \
nonstatic_field(BasicHashtable<mtInternal>, _buckets, HashtableBucket<mtInternal>*) \
nonstatic_field(BasicHashtable<mtInternal>, _free_list, BasicHashtableEntry<mtInternal>*) \
volatile_nonstatic_field(BasicHashtable<mtInternal>, _free_list, BasicHashtableEntry<mtInternal>*) \
nonstatic_field(BasicHashtable<mtInternal>, _first_free_entry, char*) \
nonstatic_field(BasicHashtable<mtInternal>, _end_block, char*) \
nonstatic_field(BasicHashtable<mtInternal>, _entry_size, int) \

31
hotspot/src/share/vm/utilities/hashtable.cpp
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 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
@ -173,6 +173,35 @@ template <MEMFLAGS F> void BasicHashtable<F>::reverse() {
}
}
template <MEMFLAGS F> void BasicHashtable<F>::BucketUnlinkContext::free_entry(BasicHashtableEntry<F>* entry) {
entry->set_next(_removed_head);
_removed_head = entry;
if (_removed_tail == NULL) {
_removed_tail = entry;
}
_num_removed++;
}
template <MEMFLAGS F> void BasicHashtable<F>::bulk_free_entries(BucketUnlinkContext* context) {
if (context->_num_removed == 0) {
assert(context->_removed_head == NULL && context->_removed_tail == NULL,
"Zero entries in the unlink context, but elements linked from " PTR_FORMAT " to " PTR_FORMAT,
p2i(context->_removed_head), p2i(context->_removed_tail));
return;
}
// MT-safe add of the list of BasicHashTableEntrys from the context to the free list.
BasicHashtableEntry<F>* current = _free_list;
while (true) {
context->_removed_tail->set_next(current);
BasicHashtableEntry<F>* old = (BasicHashtableEntry<F>*)Atomic::cmpxchg_ptr(context->_removed_head, &_free_list, current);
if (old == current) {
break;
}
current = old;
}
Atomic::add(-context->_num_removed, &_number_of_entries);
}
// Copy the table to the shared space.

22
hotspot/src/share/vm/utilities/hashtable.hpp
View File

@ -173,11 +173,11 @@ private:
// Instance variables
int _table_size;
HashtableBucket<F>* _buckets;
BasicHashtableEntry<F>* _free_list;
BasicHashtableEntry<F>* volatile _free_list;
char* _first_free_entry;
char* _end_block;
int _entry_size;
int _number_of_entries;
volatile int _number_of_entries;
protected:
@ -225,6 +225,24 @@ protected:
// Free the buckets in this hashtable
void free_buckets();
// Helper data structure containing context for the bucket entry unlink process,
// storing the unlinked buckets in a linked list.
// Also avoids the need to pass around these four members as parameters everywhere.
struct BucketUnlinkContext {
int _num_processed;
int _num_removed;
// Head and tail pointers for the linked list of removed entries.
BasicHashtableEntry<F>* _removed_head;
BasicHashtableEntry<F>* _removed_tail;
BucketUnlinkContext() : _num_processed(0), _num_removed(0), _removed_head(NULL), _removed_tail(NULL) {
}
void free_entry(BasicHashtableEntry<F>* entry);
};
// Add of bucket entries linked together in the given context to the global free list. This method
// is mt-safe wrt. to other calls of this method.
void bulk_free_entries(BucketUnlinkContext* context);
public:
int table_size() { return _table_size; }
void set_entry(int index, BasicHashtableEntry<F>* entry);