Thread.getState() very slow

[Mandy Chung]

Hi Doug, David,

I'm not aware of any CRs related to the performance of 
Thread.getState().  Like Alan said, the implementation hasn't been 
changed since JDK 5 release.  Probably the existing usage of 
Thread.getState() is not as performance sensitive as the starvation 
detector requires and thus the performance issue hasn't been noticed.

David Holmes wrote:
> Hi Doug,
>
> Doug Lea said the following on 07/11/10 21:20:
>> Also, while I'm at it, field Thread.threadStatus is suspiciously not
>> declared volatile.
>
> Agreed - technically this should be a volatile field. In practice I 
> think it only means that the window in which a stale value might be 
> seen is slightly longer. If threadStatus were exported directly it 
> might be more of an issue, but as an internal detail of getState() it 
> is not likely to be.
>
>>> Can you use Unsafe to grab the field directly, or would the 
>>> reflection make it too slow?
>>>
>>
>> That's what I'm currently doing as a sleazy workaround.
>> The sleaziness is not the Unsafe, which is basically fine
>> since this is planned for JDK code, but having to guess at startup
>> that the first value I get must be the one associated with RUNNABLE.
>
> I thought this would be fairly simple but looking deeper into this it 
> is a lot more complicated than I thought. There is a three-level 
> mapping from:
>
> OS/VM thread state -> VM ThreadStatus -> java.lang.ThreadState
>
> and it is a M:N:1 mapping
>

This is complicated than it needed to.

I vaguely remember that the current implementation considered the 
compatibility issue between different version of hotspot VM and JDK 
(e.g. older VM running on a newer JDK with a new Thread.State enum).   
Also, the java_lang_Thread::ThreadStatus in hotspot was defined as 
ordinal values that we wanted to avoid those values hardcoded in both 
hotspot and jdk.   Later it was changed to a hierarchical thread state 
represented as a bit vector (4980307) that we should have revised the 
implementation to map the bit vector directly to Thread.State enum but 
clearly the change in jdk was not done.

> At least a startup you should be able to get the current thread's 
> threadStatus and know that it represents RUNNABLE.
>
> In addition I suspect the lazy initialization may be more necessary 
> than I thought given the time at which this initialization might occur 
> during the VM initialization sequence. I'd have to experiment (once 
> I've remembered how to build regular SE Hotspot and the OpenJDK libs :) )
>
> Some input from the original implementors would be useful (Martin? 
> Mandy? ...)
>

Looking at the hotspot implementation 
(src/share/vm/classfile/javaClasses.hpp), java_lang_Thread::ThreadStatus 
is defined as a bit vector of JVMTI thread state flags:
   
http://download.java.net/jdk7/docs/platform/jvmti/jvmti.html#GetThreadState

I think making Thread.threadStatus a volatile field and using 
JVMTI_JAVA_LANG_THREAD_STATE_MASK to convert Thread.threadStatus to 
Thread.State enum would be a good solution.

> That all said, you are the best person to evaluate the performance 
> impact of any changes so it might be best if you are the one to do the 
> initial experiments - using a simple volatile + DCL for the init as a 
> first attempt perhaps.
>

It'd be great if Doug can do the experiment and evaluate the performance 
impact.  If you want me to prototype the jdk change, let me know and I 
should be able to make some time to do it next week.

Mandy

> David
>
>>
>> -Doug
>>