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
