Mercurial > hg > graal-jvmci-8
changeset 4022:db89aa49298f
7099824: G1: we should take the pending list lock before doing the remark pause
Summary: Acquire the pending list lock in the prologue method of G1's concurrent VM_Operation and release the lock in the epilogue() method. The locking/unlocking order of the pending list lock and the Heap_lock should match that in the prologue and epilogue methods of VM_GC_Operation.
Reviewed-by: tonyp, ysr
author | johnc |
---|---|
date | Thu, 20 Oct 2011 12:06:20 -0700 |
parents | 8d161913dfc3 |
children | c6a6e936dc68 |
files | src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp src/share/vm/gc_implementation/g1/vm_operations_g1.cpp src/share/vm/gc_implementation/g1/vm_operations_g1.hpp src/share/vm/gc_implementation/shared/concurrentGCThread.cpp |
diffstat | 4 files changed, 40 insertions(+), 11 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp Fri Oct 21 12:42:42 2011 -0400 +++ b/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp Thu Oct 20 12:06:20 2011 -0700 @@ -147,12 +147,8 @@ } } } while (cm()->restart_for_overflow()); + double counting_start_time = os::elapsedVTime(); - - // YSR: These look dubious (i.e. redundant) !!! FIX ME - slt()->manipulatePLL(SurrogateLockerThread::acquirePLL); - slt()->manipulatePLL(SurrogateLockerThread::releaseAndNotifyPLL); - if (!cm()->has_aborted()) { double count_start_sec = os::elapsedTime(); if (PrintGC) { @@ -175,6 +171,7 @@ } } } + double end_time = os::elapsedVTime(); _vtime_count_accum += (end_time - counting_start_time); // Update the total virtual time before doing this, since it will try @@ -335,13 +332,15 @@ clear_started(); } -// Note: this method, although exported by the ConcurrentMarkSweepThread, -// which is a non-JavaThread, can only be called by a JavaThread. -// Currently this is done at vm creation time (post-vm-init) by the -// main/Primordial (Java)Thread. -// XXX Consider changing this in the future to allow the CMS thread +// Note: As is the case with CMS - this method, although exported +// by the ConcurrentMarkThread, which is a non-JavaThread, can only +// be called by a JavaThread. Currently this is done at vm creation +// time (post-vm-init) by the main/Primordial (Java)Thread. +// XXX Consider changing this in the future to allow the CM thread // itself to create this thread? void ConcurrentMarkThread::makeSurrogateLockerThread(TRAPS) { + assert(UseG1GC, "SLT thread needed only for concurrent GC"); + assert(THREAD->is_Java_thread(), "must be a Java thread"); assert(_slt == NULL, "SLT already created"); _slt = SurrogateLockerThread::make(THREAD); }
--- a/src/share/vm/gc_implementation/g1/vm_operations_g1.cpp Fri Oct 21 12:42:42 2011 -0400 +++ b/src/share/vm/gc_implementation/g1/vm_operations_g1.cpp Thu Oct 20 12:06:20 2011 -0700 @@ -23,6 +23,7 @@ */ #include "precompiled.hpp" +#include "gc_implementation/g1/concurrentMarkThread.inline.hpp" #include "gc_implementation/g1/g1CollectedHeap.inline.hpp" #include "gc_implementation/g1/g1CollectorPolicy.hpp" #include "gc_implementation/g1/vm_operations_g1.hpp" @@ -165,6 +166,20 @@ } } +void VM_CGC_Operation::acquire_pending_list_lock() { + // The caller may block while communicating + // with the SLT thread in order to acquire/release the PLL. + ConcurrentMarkThread::slt()-> + manipulatePLL(SurrogateLockerThread::acquirePLL); +} + +void VM_CGC_Operation::release_and_notify_pending_list_lock() { + // The caller may block while communicating + // with the SLT thread in order to acquire/release the PLL. + ConcurrentMarkThread::slt()-> + manipulatePLL(SurrogateLockerThread::releaseAndNotifyPLL); +} + void VM_CGC_Operation::doit() { gclog_or_tty->date_stamp(PrintGC && PrintGCDateStamps); TraceCPUTime tcpu(PrintGCDetails, true, gclog_or_tty); @@ -180,12 +195,19 @@ } bool VM_CGC_Operation::doit_prologue() { + // Note the relative order of the locks must match that in + // VM_GC_Operation::doit_prologue() or deadlocks can occur + acquire_pending_list_lock(); + Heap_lock->lock(); SharedHeap::heap()->_thread_holds_heap_lock_for_gc = true; return true; } void VM_CGC_Operation::doit_epilogue() { + // Note the relative order of the unlocks must match that in + // VM_GC_Operation::doit_epilogue() SharedHeap::heap()->_thread_holds_heap_lock_for_gc = false; Heap_lock->unlock(); + release_and_notify_pending_list_lock(); }
--- a/src/share/vm/gc_implementation/g1/vm_operations_g1.hpp Fri Oct 21 12:42:42 2011 -0400 +++ b/src/share/vm/gc_implementation/g1/vm_operations_g1.hpp Thu Oct 20 12:06:20 2011 -0700 @@ -93,11 +93,17 @@ } }; -// Concurrent GC stop-the-world operations such as initial and final mark; +// Concurrent GC stop-the-world operations such as remark and cleanup; // consider sharing these with CMS's counterparts. class VM_CGC_Operation: public VM_Operation { VoidClosure* _cl; const char* _printGCMessage; + +protected: + // java.lang.ref.Reference support + void acquire_pending_list_lock(); + void release_and_notify_pending_list_lock(); + public: VM_CGC_Operation(VoidClosure* cl, const char *printGCMsg) : _cl(cl), _printGCMessage(printGCMsg) { }
--- a/src/share/vm/gc_implementation/shared/concurrentGCThread.cpp Fri Oct 21 12:42:42 2011 -0400 +++ b/src/share/vm/gc_implementation/shared/concurrentGCThread.cpp Thu Oct 20 12:06:20 2011 -0700 @@ -224,6 +224,8 @@ MutexLockerEx x(&_monitor, Mutex::_no_safepoint_check_flag); assert(_buffer == empty, "Should be empty"); assert(msg != empty, "empty message"); + assert(!Heap_lock->owned_by_self(), "Heap_lock owned by requesting thread"); + _buffer = msg; while (_buffer != empty) { _monitor.notify();