RFR: 8152905: hs_err file is missing gc threads

Derek White derek.white at oracle.com
Wed Mar 30 20:20:17 UTC 2016


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 hs_err file in the "Other 
>> Threads" section
>>
>> There are 4 small parts to this:
>> 1) Fix G1CollectedHeap::gc_threads_do() to also iterate over 
>> concurrent mark worker threads.
>> 2) Have Thread::print_on_error() print the thread name of NamedThreads.
>> 3) Factor out code that prints out each "Other Thread" to 
>> Threads::print_on_error().
>> 4) Call Threads::print_on_error() on every GC thread.
>>
>> BUG: 8152905: hs_err 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 (is_Named_thread()) {
>     L834:     st->print(" \"%s\"", name());
>     L835:   }
>
>         The new is_Named_thread() 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 (is_Named_thread()) {
>             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.

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::print_on_error(), because the overridden name() might allocate, 
but print_on_error() is also overridden, so it's safe after all. I'll 
add asserts in Thread::print_on_error() that it's not called on a 
JavaThread or CompilerThread.
>
>     L4496:   if (this_thread) {
>         Should not use an implied boolean. This should be:
>
>         if (this_thread != NULL) {
OK.
>
>     L4533:     bool is_current = (current == thread);
>     L4534:     found_current = found_current || is_current;
>     L4535:
>     L4536:     st->print("%s", is_current ? "=>" : "  ");
>     L4537:
>     L4538:     st->print(PTR_FORMAT, p2i(thread));
>     L4539:     st->print(" ");
>     L4540:     thread->print_on_error(st, buf, buflen);
>     L4541:     st->cr();
>
>         I think this block can refactor into:
>
>           print_on_error(thread, st, current, buf, buflen, 
> &found_current);

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