Review request for 6977034 Thread.getState() very slow

[Mandy Chung]

  On 12/6/10 2:41 PM, Eamonn McManus wrote:
> Reviewed OK!
> The constant JVMTI_THREAD_STATE_WAITING is not used but that's my only 
> minor niggle.

Will take it out.

Thanks for the review.
Mandy

> Éamonn [emcmanus]
>
> On 06/12/2010 20:26, Mandy Chung wrote:
>> Remi, Eamonn, Brian, David, Doug,
>>
>> Thanks for the feedback.
>>
>> On 12/04/10 04:22, Eamonn McManus wrote:
>>> Hi Mandy,
>>>
>>> This test:
>>>
>>>          if ((threadStatus&  JVMTI_THREAD_STATE_RUNNABLE) == 1) {
>>>
>>> is always false, since JVMTI_THREAD_STATE_RUNNABLE is 4. (NetBeans 
>>> 7.0 helpfully flags this; I'm not sure if earlier versions do.)
>>>
>>
>> Good catch.   This explains why the speed up for RUNNABLE was not as 
>> high in the microbenchmark measurement.  Correcting it shows that 
>> Thread.getState() gets 3.5X speed up on a thread in RUNNABLE state.
>>
>>> But, once corrected, I think you could use this idea further to 
>>> write a much simpler and faster method, on these lines:
>>>
>>>      public static Thread.State toThreadState(int threadStatus) {
>>>          if ((threadStatus&  JVMTI_THREAD_STATE_RUNNABLE)*!= 0*) {
>>>              return RUNNABLE;
>>>          } else if ((threadStatus&  JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER) != 0) {
>>>              return BLOCKED;
>>>          } else if ((threadStatus&  JVMTI_THREAD_STATE_WAITING_WITH_TIMEOUT) != 0) {
>>>              return TIMED_WAITING;
>>>          } else if ((threadStatus&  JVMTI_THREAD_STATE_WAITING_INDEFINITELY) != 0) {
>>>              return WAITING;
>>>          } else if ((threadStatus&  JVMTI_THREAD_STATE_TERMINATED) != 0) {
>>>              return TERMINATED;
>>>          } else {
>>>              return NEW;
>>>          }
>>>      }
>>
>> I forgot to mention in the email that I implemented this simpler 
>> approach to compare with the table lookup approach.   There were no 
>> significant difference.  I now rerun with the corrected fix (checking 
>> != 0 rather than == 1) and the table lookup approach is about 2-6% 
>> faster than the sequence of tests approach.
>>
>> I am also for the simpler approach but I post the table lookup 
>> approach as a proposed fix to get any opinion on the performance 
>> aspect with that approach.
>>
>> Given that the Fork-Join framework doesn't depend on it, I will go 
>> for a simpler approach (sequence of tests) and further tune its 
>> performance when there is a use case requiring a perf improvement.
>>
>> New webrev:
>> http://cr.openjdk.java.net/~mchung/6977034/webrev.01/
>>
>> Can you review this version?
>>
>> Thanks
>> Mandy
>>
>>> You could tweak the order of the tests based on what might be the 
>>> relative frequency of the different states but it probably isn't 
>>> worth it.
>>>
>>> Regards,
>>> Éamonn
>>>
>>> On 3/12/10 11:52 PM, Mandy Chung wrote:
>>>> Fix for 6977034: Thread.getState() very slow
>>>>
>>>> Webrev at:
>>>> http://cr.openjdk.java.net/~mchung/6977034/webrev.00/
>>>>
>>>> This is an improvement to map a Thread's threadStatus field to 
>>>> Thread.State.  The VM updates the Thread.threadStatus field 
>>>> directly at state transition with the value as defined in JVM TI 
>>>> [1].  The java.lang.Thread.getState() implementation can directly 
>>>> access the threadStatus value and do a direct lookup from an array 
>>>> of Thread.State.  The threadStatus value is a bit vector and we 
>>>> would have to create an array of a minimum of 1061 (0x425) elements 
>>>> to do direct mapping.   I took the approach to use the first 
>>>> highest order bit set to 1 in the masked threadStatus value as the 
>>>> index to the Thread.State element and only caches 32 elements 
>>>> (could be fewer).  I wrote a micro-benchmark measuring the 
>>>> Thread.getState of a thread in different state that shows 1.7X to 
>>>> 6X speedup (see below).  There is possibly some issue with my 
>>>> micro-benchmark that I didn't observe the 14X speed up as Doug did 
>>>> in his experiment.  However, I'd like to get this reviewed and 
>>>> pushed to the repository so that anyone can do more experiment on 
>>>> the performance measurement.
>>>>
>>>> Thanks
>>>> Mandy
>>>> P.S. The discussion on this thread can be found at [2] [3].
>>>>
>>>> [1] 
>>>> http://download.java.net/jdk7/docs/platform/jvmti/jvmti.html#GetThreadState
>>>> [2] 
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-July/004567.html
>>>> [3] 
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-August/004721.html
>>>>
>>>>
>>>> 	JDK 7 b120 (in ms)	With fix (in ms)	Speed up
>>>> main		46465	        22772			2.04
>>>> NEW		50676		29921			1.69
>>>> RUNNABLE	42202		14690			2.87
>>>> BLOCKED		72773		12296			5.92
>>>> WAITING		48811		13041			3.74
>>>> TIMED_WAITING	45737		12849			3.56
>>>> TERMINATED	40314		16376			2.46
>>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20101206/bcab7d35/attachment.html>