Removing intrinsic of Thread.isInterrupted() (original) (raw)

Yumin Qi yumin.qi at oracle.com
Tue Feb 25 12:41:23 PST 2014

When I am writing email I saw your email.

to answer Vladimir's question, I come up with a scenario:

When Thread is in process of interrupt call:

OSThread* osthread = thread->osthread(); osthread->set_interrupted(true); // More than one thread can get here with the same value of osthread, // resulting in multiple notifications. We do, however, want the store // to interrupted() to be visible to other threads before we post // the interrupt event. OrderAccess::release(); SetEvent(osthread->interrupt_event());

Before SetEvent, bit is set.

Now, with intrinsification, call

Thread.currentThread().isInterrupted() will return return 'true' due

to clear_int is 'false'.

 The following call
 Thread.isInterrupted() will  go to slow path, since the Event is

not set, we got a 'false' with new fix.

 Karen's suggestion is get ride of second fastpath (t ==

Thread.current() && !clear_int) so it will be

return  (t == Thread.current() && !TLS._osthread._interrupted) ?

fast : slow

Thanks Yumin Thanks Yumin

On 2/25/2014 11:56 AM, Karen Kinnear wrote:

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::isinterruped. 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

More information about the hotspot-compiler-dev mailing list