8160539: Stack frame scanning acquires DerivedPointerTableGC_lock mutex

Use lock-free stack for accumulating table entries.

Reviewed-by: tschatzl, kvn
This commit is contained in:
Kim Barrett 2019-05-16 20:14:54 -04:00
parent 989fa190a6
commit 4af7663979
4 changed files with 43 additions and 43 deletions

View file

@ -38,6 +38,7 @@
#include "runtime/handles.inline.hpp"
#include "runtime/signature.hpp"
#include "utilities/align.hpp"
#include "utilities/lockFreeStack.hpp"
#ifdef COMPILER1
#include "c1/c1_Defs.hpp"
#endif
@ -337,10 +338,6 @@ void OopMapSet::all_do(const frame *fr, const RegisterMap *reg_map,
}
#endif
#endif // !TIERED
// Protect the operation on the derived pointers. This
// protects the addition of derived pointers to the shared
// derived pointer table in DerivedPointerTable::add().
MutexLocker x(DerivedPointerTableGC_lock, Mutex::_no_safepoint_check_flag);
do {
omv = oms.current();
oop* loc = fr->oopmapreg_to_location(omv.reg(),reg_map);
@ -745,44 +742,56 @@ ImmutableOopMapSet* ImmutableOopMapSet::build_from(const OopMapSet* oopmap_set)
#if COMPILER2_OR_JVMCI
class DerivedPointerEntry : public CHeapObj<mtCompiler> {
private:
oop* _location; // Location of derived pointer (also pointing to the base)
intptr_t _offset; // Offset from base pointer
public:
DerivedPointerEntry(oop* location, intptr_t offset) { _location = location; _offset = offset; }
oop* location() { return _location; }
intptr_t offset() { return _offset; }
class DerivedPointerTable::Entry : public CHeapObj<mtCompiler> {
oop* _location; // Location of derived pointer, also pointing to base
intptr_t _offset; // Offset from base pointer
Entry* volatile _next;
static Entry* volatile* next_ptr(Entry& entry) { return &entry._next; }
public:
Entry(oop* location, intptr_t offset) :
_location(location), _offset(offset), _next(NULL) {}
oop* location() const { return _location; }
intptr_t offset() const { return _offset; }
Entry* next() const { return _next; }
typedef LockFreeStack<Entry, &next_ptr> List;
static List* _list;
};
GrowableArray<DerivedPointerEntry*>* DerivedPointerTable::_list = NULL;
DerivedPointerTable::Entry::List* DerivedPointerTable::Entry::_list = NULL;
bool DerivedPointerTable::_active = false;
bool DerivedPointerTable::is_empty() {
return Entry::_list == NULL || Entry::_list->empty();
}
void DerivedPointerTable::clear() {
// The first time, we create the list. Otherwise it should be
// empty. If not, then we have probably forgotton to call
// update_pointers after last GC/Scavenge.
assert (!_active, "should not be active");
assert(_list == NULL || _list->length() == 0, "table not empty");
if (_list == NULL) {
_list = new (ResourceObj::C_HEAP, mtCompiler) GrowableArray<DerivedPointerEntry*>(10, true); // Allocated on C heap
assert(is_empty(), "table not empty");
if (Entry::_list == NULL) {
void* mem = NEW_C_HEAP_OBJ(Entry::List, mtCompiler);
Entry::_list = ::new (mem) Entry::List();
}
_active = true;
}
// Returns value of location as an int
intptr_t value_of_loc(oop *pointer) { return cast_from_oop<intptr_t>((*pointer)); }
inline intptr_t value_of_loc(oop *pointer) {
return cast_from_oop<intptr_t>((*pointer));
}
void DerivedPointerTable::add(oop *derived_loc, oop *base_loc) {
assert(Universe::heap()->is_in_or_null(*base_loc), "not an oop");
assert(derived_loc != base_loc, "Base and derived in same location");
if (_active) {
assert(*derived_loc != (void*)base_loc, "location already added");
assert(_list != NULL, "list must exist");
assert(Entry::_list != NULL, "list must exist");
intptr_t offset = value_of_loc(derived_loc) - value_of_loc(base_loc);
// This assert is invalid because derived pointers can be
// arbitrarily far away from their base.
@ -798,21 +807,21 @@ void DerivedPointerTable::add(oop *derived_loc, oop *base_loc) {
}
// Set derived oop location to point to base.
*derived_loc = (oop)base_loc;
assert_lock_strong(DerivedPointerTableGC_lock);
DerivedPointerEntry *entry = new DerivedPointerEntry(derived_loc, offset);
_list->append(entry);
Entry* entry = new Entry(derived_loc, offset);
Entry::_list->push(*entry);
}
}
void DerivedPointerTable::update_pointers() {
assert(_list != NULL, "list must exist");
for(int i = 0; i < _list->length(); i++) {
DerivedPointerEntry* entry = _list->at(i);
assert(Entry::_list != NULL, "list must exist");
Entry* entries = Entry::_list->pop_all();
while (entries != NULL) {
Entry* entry = entries;
entries = entry->next();
oop* derived_loc = entry->location();
intptr_t offset = entry->offset();
// The derived oop was setup to point to location of base
oop base = **(oop**)derived_loc;
oop base = **(oop**)derived_loc;
assert(Universe::heap()->is_in_or_null(base), "must be an oop");
*derived_loc = (oop)(((address)base) + offset);
@ -826,13 +835,8 @@ void DerivedPointerTable::update_pointers() {
// Delete entry
delete entry;
_list->at_put(i, NULL);
}
// Clear list, so it is ready for next traversal (this is an invariant)
if (TraceDerivedPointers && !_list->is_empty()) {
tty->print_cr("--------------------------");
}
_list->clear();
assert(Entry::_list->empty(), "invariant");
_active = false;
}