提交 1bf17034 编写于 作者: T tonyp

6940310: G1: MT-unsafe calls to CM::region_stack_push() / CM::region_stack_pop()

Summary: Calling the methods region_stack_push() and region_stack_pop() concurrent is not MT-safe. The assumption is that we will only call region_stack_push() during a GC pause and region_stack_pop() during marking. Unfortunately, we also call region_stack_push() during marking which seems to be introducing subtle marking failures. This change introduces lock-based methods for pushing / popping to be called during marking.
Reviewed-by: iveresov, johnc
上级 fd77bac8
......@@ -297,6 +297,11 @@ void CMRegionStack::push(MemRegion mr) {
}
}
// Currently we do not call this at all. Normally we would call it
// during the concurrent marking / remark phases but we now call
// the lock-based version instead. But we might want to resurrect this
// code in the future. So, we'll leave it here commented out.
#if 0
MemRegion CMRegionStack::pop() {
while (true) {
// Otherwise...
......@@ -321,6 +326,41 @@ MemRegion CMRegionStack::pop() {
// Otherwise, we need to try again.
}
}
#endif // 0
void CMRegionStack::push_with_lock(MemRegion mr) {
assert(mr.word_size() > 0, "Precondition");
MutexLockerEx x(CMRegionStack_lock, Mutex::_no_safepoint_check_flag);
if (isFull()) {
_overflow = true;
return;
}
_base[_index] = mr;
_index += 1;
}
MemRegion CMRegionStack::pop_with_lock() {
MutexLockerEx x(CMRegionStack_lock, Mutex::_no_safepoint_check_flag);
while (true) {
if (_index == 0) {
return MemRegion();
}
_index -= 1;
MemRegion mr = _base[_index];
if (mr.start() != NULL) {
assert(mr.end() != NULL, "invariant");
assert(mr.word_size() > 0, "invariant");
return mr;
} else {
// that entry was invalidated... let's skip it
assert(mr.end() == NULL, "invariant");
}
}
}
bool CMRegionStack::invalidate_entries_into_cset() {
bool result = false;
......@@ -3363,7 +3403,7 @@ void CMTask::drain_region_stack(BitMapClosure* bc) {
gclog_or_tty->print_cr("[%d] draining region stack, size = %d",
_task_id, _cm->region_stack_size());
MemRegion mr = _cm->region_stack_pop();
MemRegion mr = _cm->region_stack_pop_with_lock();
// it returns MemRegion() if the pop fails
statsOnly(if (mr.start() != NULL) ++_region_stack_pops );
......@@ -3384,7 +3424,7 @@ void CMTask::drain_region_stack(BitMapClosure* bc) {
if (has_aborted())
mr = MemRegion();
else {
mr = _cm->region_stack_pop();
mr = _cm->region_stack_pop_with_lock();
// it returns MemRegion() if the pop fails
statsOnly(if (mr.start() != NULL) ++_region_stack_pops );
}
......@@ -3417,7 +3457,7 @@ void CMTask::drain_region_stack(BitMapClosure* bc) {
}
// Now push the part of the region we didn't scan on the
// region stack to make sure a task scans it later.
_cm->region_stack_push(newRegion);
_cm->region_stack_push_with_lock(newRegion);
}
// break from while
mr = MemRegion();
......
......@@ -252,9 +252,19 @@ public:
// with other "push" operations (no pops).
void push(MemRegion mr);
#if 0
// This is currently not used. See the comment in the .cpp file.
// Lock-free; assumes that it will only be called in parallel
// with other "pop" operations (no pushes).
MemRegion pop();
#endif // 0
// These two are the implementations that use a lock. They can be
// called concurrently with each other but they should not be called
// concurrently with the lock-free versions (push() / pop()).
void push_with_lock(MemRegion mr);
MemRegion pop_with_lock();
bool isEmpty() { return _index == 0; }
bool isFull() { return _index == _capacity; }
......@@ -540,6 +550,10 @@ public:
// Manipulation of the region stack
bool region_stack_push(MemRegion mr) {
// Currently we only call the lock-free version during evacuation
// pauses.
assert(SafepointSynchronize::is_at_safepoint(), "world should be stopped");
_regionStack.push(mr);
if (_regionStack.overflow()) {
set_has_overflown();
......@@ -547,7 +561,33 @@ public:
}
return true;
}
#if 0
// Currently this is not used. See the comment in the .cpp file.
MemRegion region_stack_pop() { return _regionStack.pop(); }
#endif // 0
bool region_stack_push_with_lock(MemRegion mr) {
// Currently we only call the lock-based version during either
// concurrent marking or remark.
assert(!SafepointSynchronize::is_at_safepoint() || !concurrent(),
"if we are at a safepoint it should be the remark safepoint");
_regionStack.push_with_lock(mr);
if (_regionStack.overflow()) {
set_has_overflown();
return false;
}
return true;
}
MemRegion region_stack_pop_with_lock() {
// Currently we only call the lock-based version during either
// concurrent marking or remark.
assert(!SafepointSynchronize::is_at_safepoint() || !concurrent(),
"if we are at a safepoint it should be the remark safepoint");
return _regionStack.pop_with_lock();
}
int region_stack_size() { return _regionStack.size(); }
bool region_stack_overflow() { return _regionStack.overflow(); }
bool region_stack_empty() { return _regionStack.isEmpty(); }
......
......@@ -70,6 +70,7 @@ Monitor* FullGCCount_lock = NULL;
Monitor* CMark_lock = NULL;
Monitor* ZF_mon = NULL;
Monitor* Cleanup_mon = NULL;
Mutex* CMRegionStack_lock = NULL;
Mutex* SATB_Q_FL_lock = NULL;
Monitor* SATB_Q_CBL_mon = NULL;
Mutex* Shared_SATB_Q_lock = NULL;
......@@ -167,6 +168,7 @@ void mutex_init() {
def(CMark_lock , Monitor, nonleaf, true ); // coordinate concurrent mark thread
def(ZF_mon , Monitor, leaf, true );
def(Cleanup_mon , Monitor, nonleaf, true );
def(CMRegionStack_lock , Mutex, leaf, true );
def(SATB_Q_FL_lock , Mutex , special, true );
def(SATB_Q_CBL_mon , Monitor, nonleaf, true );
def(Shared_SATB_Q_lock , Mutex, nonleaf, true );
......
......@@ -63,6 +63,7 @@ extern Monitor* FullGCCount_lock; // in support of "concurrent" f
extern Monitor* CMark_lock; // used for concurrent mark thread coordination
extern Monitor* ZF_mon; // used for G1 conc zero-fill.
extern Monitor* Cleanup_mon; // used for G1 conc cleanup.
extern Mutex* CMRegionStack_lock; // used for protecting accesses to the CM region stack
extern Mutex* SATB_Q_FL_lock; // Protects SATB Q
// buffer free list.
extern Monitor* SATB_Q_CBL_mon; // Protects SATB Q
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册