Code review request: three native memory tracking related bugs (original) (raw)

Zhengyu Gu zhengyu.gu at oracle.com
Mon Jul 16 07:39:47 PDT 2012

Hi David,

On 7/14/2012 2:57 AM, David Holmes wrote:

On 14/07/2012 5:43 AM, Zhengyu Gu wrote:

7181989: NMT ON: Assertion failure when NMT checks thread's native CR: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7181989 Webrev: http://cr.openjdk.java.net/~zgu/7181989/webrev.00/

We try to assert Thread's stack base to ensure that Thread::recordstackbaseandsize() to record native stack to NMT, but there is scenario that the thread fails to start, which no native stack is created and should not be asserted. I'm not really clear on why we need to refer to the assert inside stackbase(). If any started thread fails to setup the stackbase and stacksize then it will likely crash well prior to the Thread destructor. It seems to me that all that is needed here is to only call MemTracker::recordvirtualmemoryrelease for a thread that actually started, and for that you can simplify the check to: if (osthr != NULL && osthr->getstate() > INITIALIZED) and so the whole thing can be expressed simply as: // Update NMT on Thread destruction, but only if the Thread // actually started OSThread* osthr = osthread(); if (osthr != NULL && osthr->getstate() > INITIALIZED) { MemTracker::recordvirtualmemoryrelease( (stackbase() - stacksize()), stacksize(), this); } Fair enough, since the assertion has yet caught anything, I assume that threads are calling record_stack_base_and_size(), so the assertion is not necessary.

7181986: NMT ON: Assertion failure when running jdi ExpiredRequestDeletionTest CR: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7181986 Webrev: http://cr.openjdk.java.net/~zgu/7181986/webrev.00/

This is a racing condition when C/C++ runtime exit handler is ran before NMT worker thread exits. The exit handler calls querylock's destructor while NMT worker thread is still holding it. The fix is to make querylock a heap object, instead of static object, but the drawback is that, it does not seem that querylock can be safely deleted. Also, I reassigned MemSnaphot lock and query lock, so they participate in the deadlock detection logic. src/share/vm/services/memSnapshot.cpp We don't use std::nothrow when allocating other VM mutexes, and you are not checking for allocation failures. We do check "_lock == NULL" in out_of_memory(), which is not in the webrev.

Can't comment on whether the 'level' assigned to the mutexes is appropriate - only testing and time will tell.

7182543: NMT ON: Aggregate a few NMT related bugs CRhttp://bugs.sun.com/bugdatabase/viewbug.do?bugid=7182543 Webrev: http://cr.openjdk.java.net/~zgu/7182543/webrev.00/

- Fixed generationsinused calculation - Wait MemRecorder instance count to drop to zero before completely shutdown NMT So previously the instancecount was only present in debug. Is this likely to be a contention bottleneck? - Added assertion for JavaThread in threadblocked state. Is the check for a safepoint needed? I'm assuming this assertion is trying to verify that threads in threadblocked don't do allocation/deallocation (though I'm unclear why the constraint exists??). Whether a safepoint is presently active or not seems irrelevant ?? The initial assumption was that threads in _thread_blocked don't allocate/deallocate memory, but it is not the case ...

Talked to Karen, the conclusion is that, NMT should just use ThreadCritical to block the threads in _thread_blocked state.

Thanks,

-Zhengyu

Cheers, David

Thanks, -Zhengyu

More information about the hotspot-dev mailing list