Removing intrinsic of Thread.isInterrupted()

[Karen Kinnear]

Vladimir,

I updated the bug to reflect the code review email thread between Yumin, myself
and David Holmes.

To the best of my understanding there is a potential timing hole here, with the clearInterrupted
case (study the webrev for the fix), i.e. in the isInterrupted(false) case.

What I proposed was that we could keep the intrinsic quick check for isInterrupted bit, but not
the logic for the isInterrupted(false) - unless you want to change it to add the Windows
WaitForSingleObject call - which I assume removes the benefit of the intrinsic.

Does that make sense to you?
thanks,
Karen

On Feb 25, 2014, at 2:14 PM, Vladimir Kozlov wrote:

> Yumin,
> 
> On 2/24/14 5:46 PM, Yumin Qi wrote:
>> Hi, Compiler team
>> 
>>   I worked on this bug: 6498581:ThreadInterruptTest3 produces wrong
>> output on Windows.  This is a problem thread sleep wakes up spuriously
>> due to a race condition between os::interrupt and os::is_interruped.
>> Detail please see bug comments.
>> 
>>  https://bugs.openjdk.java.net/browse/JDK-6498581
>> 
>>   The fix is (without removing intrinsic, but intend to remove it) :
>>   http://cr.openjdk.java.net/~minqi/6498581/webrev00/
>> 
>>   One problem is that Thread.isInterrupted() is intrinsic and there is
>> chance that code like
>> 
>>   boolean a = Thread.currentThread().isInterrupted();
>>   boolean b = Thread.interrupted();
>> 
>>   Will get different value. (fast/slow path)
> 
> How you come to this conclusion? You my be mistaken. We intrinsify native boolean isInterrupted(boolean ClearInterrupted) and not java method: isInterrupted().
> 
> Also you are comparing different code which is nothing to do with intrinsic:
> 
> isInterrupted() passes ClearInterrupted=false:
> 
>    public boolean isInterrupted() {
>        return isInterrupted(false);
>    }
> 
> when interrupted() passes 'true':
> 
>    public static boolean interrupted() {
>        return currentThread().isInterrupted(true);
>    }
> 
> Both method calls native isInterrupted(bool) which is intrinsified. So both calls intrinsic. There should be no difference.
> 
> From performance point of view, as Aleksey pointed, there is huge difference. We can't remove intrinsic.
> 
> Thanks,
> Vladimir
> 
>>   I tried to remove the intrinsic code and done a test using following
>> code. The result showed there is no difference by removing the intrinsic
>> of Thread.isInterrupted().
>> 
>> // test performance of removing Thread.isInterrupted() inlining
>> public class TestThreadInterrupted {
>>     public static void main(String... args) {
>>         Thread t = new Thread () {
>>             public void run() {
>>                 boolean isInt = false;
>>                 while (!isInt) {
>>                     try {
>>                         Thread.sleep(30);
>>                     } catch (InterruptedException ie) {
>>                         isInt = true;
>>                     }
>>                 }
>>             }
>>         };
>> 
>>         t.start();
>>         // run
>>         long start, finish, isum = 0L, osum = 0L;
>>         int  NUM = 20000;
>>         for (int j = 0; j < 100; j++) {
>>         isum = 0L;
>>         for (int i = 0; i < NUM; i++) {
>>             start = System.currentTimeMillis();
>>             t.isInterrupted();
>>             finish = System.currentTimeMillis();
>>             isum += (finish - start);
>>         }
>> 
>>         System.out.println("Total cost of " + NUM + " calls is " + isum
>> + " ms");
>>         osum += isum;
>>         }
>>         System.out.println("Average " + osum/100 + " ms");
>>         t.interrupt();
>>         try {
>>             t.join();
>>         } catch (InterruptedException e) {}
>>     }
>> }
>> 
>> And found there is no difference on Solaris-x64/sparcv9, Windows(32/64),
>> linux(32/64) before and after the removing of intrinsic
>> Thread.isInterrupted().
>> 
>> Should I remove the intrinsic?
>> 
>> Data (no major difference  for both with/without intrinsic):
>> 
>> 1)windows :
>> ....
>> Total cost of 20000 calls is 2 ms
>> Total cost of 20000 calls is 1 ms
>> Total cost of 20000 calls is 1 ms
>> Total cost of 20000 calls is 1 ms
>> Total cost of 20000 calls is 2 ms
>> Total cost of 20000 calls is 1 ms
>> Total cost of 20000 calls is 2 ms
>> Total cost of 20000 calls is 2 ms
>> Total cost of 20000 calls is 2 ms
>> Total cost of 20000 calls is 0 ms
>> Total cost of 20000 calls is 2 ms
>> Total cost of 20000 calls is 2 ms
>> Average 1 ms
>> 
>> 2) Solaris-x64
>> ....
>> Total cost of 20000 calls is 3 ms
>> Total cost of 20000 calls is 1 ms
>> Total cost of 20000 calls is 4 ms
>> Total cost of 20000 calls is 6 ms
>> Total cost of 20000 calls is 6 ms
>> Total cost of 20000 calls is 5 ms
>> Total cost of 20000 calls is 7 ms
>> Total cost of 20000 calls is 5 ms
>> Total cost of 20000 calls is 5 ms
>> Total cost of 20000 calls is 1 ms
>> Total cost of 20000 calls is 3 ms
>> Total cost of 20000 calls is 2 ms
>> Total cost of 20000 calls is 3 ms
>> Total cost of 20000 calls is 3 ms
>> Total cost of 20000 calls is 5 ms
>> Total cost of 20000 calls is 4 ms
>> Total cost of 20000 calls is 4 ms
>> Total cost of 20000 calls is 7 ms
>> Average 4 ms
>> 
>> 3) Linux:
>> 
>> ....
>> Total cost of 20000 calls is 30 ms
>> Total cost of 20000 calls is 29 ms
>> Total cost of 20000 calls is 26 ms
>> Total cost of 20000 calls is 26 ms
>> Total cost of 20000 calls is 26 ms
>> Total cost of 20000 calls is 24 ms
>> Total cost of 20000 calls is 29 ms
>> Total cost of 20000 calls is 25 ms
>> Total cost of 20000 calls is 20 ms
>> Average 24 ms
>> 
>> 
>> Thanks
>> Yumin