Looks good! (and thanks for adding the Java thread id to the output)
/Staffan On 17 sep 2012, at 05:53, David Holmes <david.hol...@oracle.com> wrote: > I've updated the fix to include the missing/extra newlines in the thread > listing that was reported. Seems to be okay now for: > - G1 > - CMS > - ParNew > - Serial (trivially so as there is no output :) ) > > New Webrev at: > > http://cr.openjdk.java.net/~dholmes/7194254/webrev/ > > (Sorry I messed up the versioning of the webrev.) > > Now the repos are open again could I please get an additional reviewer from > runtime or serviceability so I can push this to hotspot-rt. > > Thanks, > David > > > On 7/09/2012 7:14 PM, David Holmes wrote: >> On 7/09/2012 6:21 PM, Dmytro Sheyko wrote: >>> David, >>> >>> Maybe it makes sense to do some little corrections in format specifiers: >>> >>> st->print("#" INT64_FORMAT " ", java_lang_Thread::thread_id(thread_oop)); >>> if (java_lang_Thread::is_daemon(thread_oop)) st->print("daemon "); >>> st->print("prio=" INT32_FORMAT " ", >>> java_lang_Thread::priority(thread_oop)); >>> >>> instead of >>> >>> st->print("#%ld ", java_lang_Thread::thread_id(thread_oop)); >>> if (java_lang_Thread::is_daemon(thread_oop)) st->print("daemon "); >>> st->print("prio=%d ", java_lang_Thread::priority(thread_oop)); >> >> I confused myself over the correctness of %ld versus your original %llx >> - sorry. The %d is fine and is used elsewhere when printing the priority. >> >> Webrev updated at same location. >> >> Thanks, >> David >> >>> Thanks, >>> Dmytro >>> >>> >>> > Date: Fri, 7 Sep 2012 16:54:32 +1000 >>> > From: david.hol...@oracle.com >>> > To: hotspot-...@openjdk.java.net >>> > CC: dmytro_she...@hotmail.com; serviceability-dev@openjdk.java.net >>> > Subject: RFR: 7194254 jstack reports wrong thread priorities >>> > >>> > This is a formal request for review for the patch contributed by Dymtro >>> > Sheyko as discussed previously here: >>> > >>> > >>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2012-August/006376.html >>> >>> > >>> > I am one reviewer of course. >>> > >>> > The webrev is here: >>> > >>> > http://cr.openjdk.java.net/~dholmes/7194254/webrev.v1/ >>> > >>> > The fix has two components: >>> > >>> > 1. It fixes a bug in os::get_priority that assumed a more positive >>> > integer was always higher priority than a less positive one. >>> > >>> > 2. It addresses the problem that os::get_priority is often inexact when >>> > desiring the Java thread priority (because the mapping from Java >>> > priority to OS priority is often M:1) by not using it in >>> > Threads::print_on. Instead Threads::print_on will always report the >>> > native OS priority, and JavaThread::print_on() will print the >>> > java.lang.Thread.getId() value together with the >>> > java.lang.Thread.getPriority() value. >>> > >>> > This change in output affects all stackdumps including crash logs and >>> > thread dumps (including those shown by jstack). >>> > >>> > There is also a test program to check jstack output. I'll be doing some >>> > additional validation while the RFR is in progress. >>> > >>> > Thanks, >>> > David
