Removing intrinsic of Thread.isInterrupted()
David Holmes
david.holmes at oracle.com
Sun Mar 2 20:50:37 PST 2014
On 26/02/2014 8:50 AM, Vladimir Kozlov wrote:
> Thank you, Karen and Yumin, for explanation.
>
> I understand the problem now.
>
> Should we remove (put under #ifndef) fast path "if (TLS._interrupted &&
> !clear_int) return true;" only for Windows or for other platforms too?
I assume this was resolved elsewhere but the problem is only on Windows.
David
-----
> Thanks,
> Vladimir
>
> On 2/25/14 12:41 PM, Yumin Qi wrote:
>> 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::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
>>
More information about the hotspot-compiler-dev
mailing list