jstack reports wrong thread priorities
David Holmes
david.holmes at oracle.com
Sun Sep 2 22:44:58 PDT 2012
Hi Dmytro,
I've moved 7194254 to the runtime area rather than serviceability as
this is a VM issue not a jstack issue.
The use of priority in VM_Operation was to allow prioritization of VM
operation requests, but this is an unused facility.
There is obviously an issue here, but there are some aspects of this
that feel awkward to me. A balanced API has a getX to match a setX so it
seems wrong to simply get rid of some of these methods rather than
changing/fixing their semantics. os::set_priority is a wrapper around
os::set_native_priority and has to map from a Java priority to a native
one. Similarly os::get_priority is a wrapper around
os::get_native_priority, and it has to convert from a native priority to
a Java priority. It just turns out that the mapping is buggy in one case
(where lesser numeric priority means higher priority) and imprecise when
the mapping was a relation rather than a function. I don't think that in
itself is a reason to kill the function - you just need to be careful
where/how you use it.
I'm more inclined to go with your original patch, of fixing the bug in
os::get_priority and changing print_on to report the native priority and
java_priority (which seemed to get lost in the second patch).
Webrev of that original patch here:
http://cr.openjdk.java.net/~dholmes/7194254/webrev.v1/
Though the format specifier %lld needs changing - %lx seems to be used
most commonly elsewhere for thread_id.
Thanks,
David
On 1/09/2012 3:46 AM, Dmytro Sheyko wrote:
>
> Put new version of patch here: https://bugs.openjdk.java.net/show_bug.cgi?id=100275
>
>> From: dmytro_sheyko at hotmail.com
>> To: david.holmes at oracle.com; hotspot-dev at openjdk.java.net
>> Subject: RE: jstack reports wrong thread priorities
>> Date: Fri, 31 Aug 2012 19:50:40 +0300
>>
>>
>> Hi,
>>
>> I tried to figure out where os::get_priority is also used. I can see that besides Thread::print_on it is used in VMThread::execute,
>> where thread priority is stored in VM_Operation using set_calling_thread. However stored priority is used nowhere. Therefore I believe that
>> os::get_priority (with Thread::get_priority) can be removed. For JavaThread we can use java_priority(), for non java thread we should
>> better use native priority rather than rough estimate of its java priority.
>>
>> Thanks,
>> Dmytro
>>
>>> Date: Tue, 28 Aug 2012 09:56:35 +1000
>>> From: david.holmes at oracle.com
>>> To: dmytro_sheyko at hotmail.com
>>> CC: hotspot-dev at openjdk.java.net
>>> Subject: Re: jstack reports wrong thread priorities
>>>
>>> Hi Dmytro,
>>>
>>> Native priorities are a bit of a mess as you can tell. I like your
>>> suggestion of reporting native priority plus Java priority distinctly,
>>> and without this complex mapping scheme. But I think part of the problem
>>> is that for some threads the priority is only manipulated at the native
>>> level and so the Java priority never gets updated.
>>>
>>> The patch for os.cpp get_priority seems reasonable.
>>>
>>> David
>>>
>>> On 28/08/2012 1:46 AM, Dmytro Sheyko wrote:
>>>>
>>>>
>>>>
>>>>
>>>> Hi,
>>>>
>>>> I noticed that jstack on Linux for all threads always shows "prio=10" (which is wrong especially for threads that have NORM_PRIORITY, i.e. 5).
>>>> It seems that os::get_priority() mistakenly assumes that for native priorities greater value corresponds to higher priority.
>>>> This is not true for niceness, which is used as native priority for linux threads (where less value corresponds to higher priority).
>>>> Thus os::get_priority() incorrectly translates native to java priority.
>>>>
>>>> Following patch fixes the problem (on Linux):
>>>>
>>>> diff -r de2aa86e037d src/share/vm/runtime/os.cpp
>>>> --- a/src/share/vm/runtime/os.cpp Thu Aug 23 12:27:33 2012 -0700
>>>> +++ b/src/share/vm/runtime/os.cpp Mon Aug 27 17:52:09 2012 +0300
>>>> @@ -208,7 +208,12 @@
>>>> OSReturn ret = get_native_priority(thread,&os_prio);
>>>> if (ret != OS_OK) return ret;
>>>>
>>>> - for (p = MaxPriority; p> MinPriority&& java_to_os_priority[p]> os_prio; p--) ;
>>>> + if (java_to_os_priority[MaxPriority]> java_to_os_priority[MinPriority]) {
>>>> + for (p = MaxPriority; p> MinPriority&& java_to_os_priority[p]> os_prio; p--) ;
>>>> + } else {
>>>> + // niceness values are in reverse order
>>>> + for (p = MaxPriority; p> MinPriority&& java_to_os_priority[p]< os_prio; p--) ;
>>>> + }
>>>> priority = (ThreadPriority)p;
>>>> return OS_OK;
>>>> }
>>>>
>>>>
>>>> However jstack is still inaccurate about priorities on Windows. It shows "prio=6" for threads that have NORM_PRIORITY.
>>>> The cause of such inaccuracy is that several java priorities are mapped to the same native priority. (E.g. 5 and 6 are mapped to THREAD_PRIORITY_NORMAL)
>>>> Therefore I believe that instead of trying to figure out java priority by native priority we should better:
>>>> 1. report native priority "as is" for all threads
>>>> 2. report java priority only for java threads using "priority" field.
>>>>
>>>> Propose following patch:
>>>>
>>>> diff -r de2aa86e037d src/share/vm/runtime/thread.cpp
>>>> --- a/src/share/vm/runtime/thread.cpp Thu Aug 23 12:27:33 2012 -0700
>>>> +++ b/src/share/vm/runtime/thread.cpp Mon Aug 27 17:52:09 2012 +0300
>>>> @@ -820,7 +820,11 @@
>>>> void Thread::print_on(outputStream* st) const {
>>>> // get_priority assumes osthread initialized
>>>> if (osthread() != NULL) {
>>>> - st->print("prio=%d tid=" INTPTR_FORMAT " ", get_priority(this), this);
>>>> + int os_prio;
>>>> + if (os::get_native_priority(this,&os_prio) == OS_OK) {
>>>> + st->print("os_prio=%d ", os_prio);
>>>> + }
>>>> + st->print("tid=" INTPTR_FORMAT " ", this);
>>>> osthread()->print_on(st);
>>>> }
>>>> debug_only(if (WizardMode) print_owned_locks_on(st);)
>>>> @@ -2710,7 +2714,11 @@
>>>> void JavaThread::print_on(outputStream *st) const {
>>>> st->print("\"%s\" ", get_thread_name());
>>>> oop thread_oop = threadObj();
>>>> - if (thread_oop != NULL&& java_lang_Thread::is_daemon(thread_oop)) st->print("daemon ");
>>>> + if (thread_oop != NULL) {
>>>> + st->print("#%lld ", 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));
>>>> + }
>>>> Thread::print_on(st);
>>>> // print guess for valid stack memory region (assume 4K pages); helps lock debugging
>>>> st->print_cr("[" INTPTR_FORMAT "]", (intptr_t)last_Java_sp()& ~right_n_bits(12));
>>>>
>>>> PS
>>>> just filled bug report for the issue:
>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7194254
>>>>
>>>> Thanks,
>>>> Dmytro
>>>>
>>>>
>>
>
More information about the hotspot-dev
mailing list