RE: jstack reports wrong thread priorities‏

[Dmytro Sheyko]

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