RFR: 7194254 jstack reports wrong thread priorities

[Frederic Parain]

Looks good.

Fred

On 09/17/12 05:53 AM, David Holmes 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.holmes at oracle.com
>>> > To: hotspot-dev at openjdk.java.net
>>> > CC: dmytro_sheyko at hotmail.com; serviceability-dev at 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

-- 
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at Oracle.com