RE: jstack reports wrong thread priorities‏

[Dmytro Sheyko]

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
> >
> >