7182543: NMT ON: Aggregate a few NMT related bugs
Summary: 1) Fixed MemTrackWorker::generations_in_used() calculation 2) Ensured NMT not to leak memory recorders after shutdown 3) Used ThreadCritical to block safepoint safe threads
Reviewed-by: acorn, coleenp, dholmes, kvn
--- a/hotspot/src/share/vm/services/memRecorder.cpp Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/src/share/vm/services/memRecorder.cpp Thu Jul 19 09:05:42 2012 -0400
@@ -45,11 +45,11 @@
}
-debug_only(volatile jint MemRecorder::_instance_count = 0;)
+volatile jint MemRecorder::_instance_count = 0;
MemRecorder::MemRecorder() {
assert(MemTracker::is_on(), "Native memory tracking is off");
- debug_only(Atomic::inc(&_instance_count);)
+ Atomic::inc(&_instance_count);
debug_only(set_generation();)
if (MemTracker::track_callsite()) {
@@ -83,9 +83,7 @@
delete _next;
}
-#ifdef ASSERT
Atomic::dec(&_instance_count);
-#endif
}
// Sorting order:
--- a/hotspot/src/share/vm/services/memRecorder.hpp Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/src/share/vm/services/memRecorder.hpp Thu Jul 19 09:05:42 2012 -0400
@@ -249,9 +249,9 @@
SequencedRecordIterator pointer_itr();
- public:
+ protected:
// number of MemRecorder instance
- debug_only(static volatile jint _instance_count;)
+ static volatile jint _instance_count;
private:
// sorting function, sort records into following order
--- a/hotspot/src/share/vm/services/memTrackWorker.hpp Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/src/share/vm/services/memTrackWorker.hpp Thu Jul 19 09:05:42 2012 -0400
@@ -67,7 +67,7 @@
NOT_PRODUCT(int _last_gen_in_use;)
inline int generations_in_use() const {
- return (_tail <= _head ? (_head - _tail + 1) : (MAX_GENERATIONS - (_tail - _head) + 1));
+ return (_tail >= _head ? (_tail - _head + 1) : (MAX_GENERATIONS - (_head - _tail) + 1));
}
};
--- a/hotspot/src/share/vm/services/memTracker.cpp Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/src/share/vm/services/memTracker.cpp Thu Jul 19 09:05:42 2012 -0400
@@ -351,21 +351,17 @@
}
if (thread != NULL) {
-#ifdef ASSERT
- // cause assertion on stack base. This ensures that threads call
- // Thread::record_stack_base_and_size() method, which will create
- // thread native stack records.
- thread->stack_base();
-#endif
- // for a JavaThread, if it is running in native state, we need to transition it to
- // VM state, so it can stop at safepoint. JavaThread running in VM state does not
- // need lock to write records.
if (thread->is_Java_thread() && ((JavaThread*)thread)->is_safepoint_visible()) {
- if (((JavaThread*)thread)->thread_state() == _thread_in_native) {
- ThreadInVMfromNative trans((JavaThread*)thread);
- create_record_in_recorder(addr, flags, size, pc, thread);
+ JavaThread* java_thread = static_cast<JavaThread*>(thread);
+ JavaThreadState state = java_thread->thread_state();
+ if (SafepointSynchronize::safepoint_safe(java_thread, state)) {
+ // JavaThreads that are safepoint safe, can run through safepoint,
+ // so ThreadCritical is needed to ensure no threads at safepoint create
+ // new records while the records are being gathered and the sequence number is changing
+ ThreadCritical tc;
+ create_record_in_recorder(addr, flags, size, pc, java_thread);
} else {
- create_record_in_recorder(addr, flags, size, pc, thread);
+ create_record_in_recorder(addr, flags, size, pc, java_thread);
}
} else {
// other threads, such as worker and watcher threads, etc. need to
@@ -390,10 +386,9 @@
// write a record to proper recorder. No lock can be taken from this method
// down.
void MemTracker::create_record_in_recorder(address addr, MEMFLAGS flags,
- size_t size, address pc, Thread* thread) {
- assert(thread == NULL || thread->is_Java_thread(), "wrong thread");
+ size_t size, address pc, JavaThread* thread) {
- MemRecorder* rc = get_thread_recorder((JavaThread*)thread);
+ MemRecorder* rc = get_thread_recorder(thread);
if (rc != NULL) {
rc->record(addr, flags, size, pc);
}
@@ -460,17 +455,18 @@
}
}
_sync_point_skip_count = 0;
- // walk all JavaThreads to collect recorders
- SyncThreadRecorderClosure stc;
- Threads::threads_do(&stc);
-
- _thread_count = stc.get_thread_count();
- MemRecorder* pending_recorders = get_pending_recorders();
-
{
// This method is running at safepoint, with ThreadCritical lock,
// it should guarantee that NMT is fully sync-ed.
ThreadCritical tc;
+
+ // walk all JavaThreads to collect recorders
+ SyncThreadRecorderClosure stc;
+ Threads::threads_do(&stc);
+
+ _thread_count = stc.get_thread_count();
+ MemRecorder* pending_recorders = get_pending_recorders();
+
if (_global_recorder != NULL) {
_global_recorder->set_next(pending_recorders);
pending_recorders = _global_recorder;
@@ -486,8 +482,6 @@
// now, it is the time to shut whole things off
if (_state == NMT_final_shutdown) {
- _tracking_level = NMT_off;
-
// walk all JavaThreads to delete all recorders
SyncThreadRecorderClosure stc;
Threads::threads_do(&stc);
@@ -499,8 +493,16 @@
_global_recorder = NULL;
}
}
-
- _state = NMT_shutdown;
+ MemRecorder* pending_recorders = get_pending_recorders();
+ if (pending_recorders != NULL) {
+ delete pending_recorders;
+ }
+ // try at a later sync point to ensure MemRecorder instance drops to zero to
+ // completely shutdown NMT
+ if (MemRecorder::_instance_count == 0) {
+ _state = NMT_shutdown;
+ _tracking_level = NMT_off;
+ }
}
}
--- a/hotspot/src/share/vm/services/memTracker.hpp Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/src/share/vm/services/memTracker.hpp Thu Jul 19 09:05:42 2012 -0400
@@ -326,7 +326,7 @@
static void create_memory_record(address addr, MEMFLAGS type,
size_t size, address pc, Thread* thread);
static void create_record_in_recorder(address addr, MEMFLAGS type,
- size_t size, address pc, Thread* thread);
+ size_t size, address pc, JavaThread* thread);
private:
// global memory snapshot