Removing intrinsic of Thread.isInterrupted()
Vladimir Kozlov
vladimir.kozlov at oracle.com
Tue Feb 25 14:50:59 PST 2014
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?
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