RFR: 8152905: hs_err file is missing gc threads (original) (raw)

Daniel D. Daugherty daniel.daugherty at oracle.com
Wed Mar 30 20:30:01 UTC 2016


On 3/30/16 2:20 PM, Derek White wrote:

Hi Dan,

On 3/30/16 11:44 AM, Daniel D. Daugherty wrote: On 3/29/16 9:32 PM, Derek White wrote:

Summary: List the GC threads in the hserr file in the "Other Threads" section

There are 4 small parts to this: 1) Fix G1CollectedHeap::gcthreadsdo() to also iterate over concurrent mark worker threads. 2) Have Thread::printonerror() print the thread name of NamedThreads. 3) Factor out code that prints out each "Other Thread" to Threads::printonerror(). 4) Call Threads::printonerror() on every GC thread. BUG: 8152905: hserr file is missing gc threads <https://bugs.openjdk.java.net/browse/JDK-8152905> WEBREV: http://cr.openjdk.java.net/~drwhite/8152905/webrev.01/ src/share/vm/runtime/thread.hpp No comments. src/share/vm/runtime/thread.cpp L831: else st->print("Thread"); L832: L833: if (isNamedthread()) { L834: st->print(" "%s"", name()); L835: } The new isNamedthread() check is currently made for every thread type, but only applies to the else-statement on L831. That else-statement should change into an else-block. Perhaps: else { st->print("Thread"); if (isNamedthread()) { st->print(" "%s"", name()); } } The intention really is to print the name for every thread type. For the GC threads, they often have the same type, but different purposes reflected in the name. For example: 0x00007f77b807e800 ConcurrentGCThread "G1 Main Marker" [stack: 0x00007f779ca7d000,0x00007f779cb7e000] [id=25865] 0x00007f77b8081000 ConcurrentGCThread "G1 Marker#0" [stack: 0x00007f779c97c000,0x00007f779ca7d000] [id=25866] 0x00007f77b806a000 ConcurrentGCThread "G1 Refine#0" [stack: 0x00007f779cee5000,0x00007f779cfe6000] [id=25863] 0x00007f77b806c000 ConcurrentGCThread "G1 Young RemSet Sampling" [stack: 0x00007f779cde4000,0x00007f779cee5000] [id=25864] 0x00007f77b8296000 ConcurrentGCThread "G1 StrDedup" [stack: 0x00007f770c757000,0x00007f770c858000] [id=25871] I can add explicit braces to the if-else-tree to make it clear that this is not an oversight.

No need for the explicit braces. I simply misunderstood what you were trying to do here.

Dan

On a related note, I realized after I sent this that it's not safe to call thread->name() on a JavaThread or CompilerThread in Thread::printonerror(), because the overridden name() might allocate, but printonerror() is also overridden, so it's safe after all. I'll add asserts in Thread::printonerror() that it's not called on a JavaThread or CompilerThread.

L4496: if (thisthread) { Should not use an implied boolean. This should be: if (thisthread != NULL) { OK. L4533: bool iscurrent = (current == thread); L4534: foundcurrent = foundcurrent || iscurrent; L4535: L4536: st->print("%s", iscurrent ? "=>" : " "); L4537: L4538: st->print(PTRFORMAT, p2i(thread)); L4539: st->print(" "); L4540: thread->printonerror(st, buf, buflen); L4541: st->cr(); I think this block can refactor into: printonerror(thread, st, current, buf, buflen, &foundcurrent); Oh, that works! src/share/vm/gc/g1/g1CollectedHeap.cpp No comments. src/share/vm/gc/g1/g1ConcurrentMark.cpp No comments. src/share/vm/gc/g1/g1ConcurrentMark.hpp No comments. Dan Thanks for the review! I'll post an updated webrev once tested. - Derek

TESTING: - Tested "java -XX:ErrorHandlerTest=1 -version" on all collectors. - jprt Thanks, - Derek



More information about the hotspot-gc-dev mailing list