'Re: RFR 8064694: Kitchensink: WaitForMultipleObjects failed in hotspot\src\os\windows\vm\os_windows.'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR 8064694: Kitchensink: WaitForMultipleObjects failed in hotspot\src\os\windows\vm\os_windows.
From:       David Holmes <david.holmes () oracle ! com>
Date:       2014-11-24 5:07:19
Message-ID: 5472BD07.6050405 () oracle ! com
[Download RAW message or body]

On 20/11/2014 10:51 PM, Ivan Gerasimov wrote:
> Thank you Daniel!
>
> David, are you still Okay with the updated webrev?

Yes.

Thanks,
David

> Comparing to the previous one, I've added setting the priority of the
> current thread at the line 3880 and changed the priority level to
> from HIGHEST to ABOVE_NORMAL.
>
> Sincerely yours,
> Ivan
>
> On 18.11.2014 18:27, Daniel D. Daugherty wrote:
>> > http://cr.openjdk.java.net/~igerasim/8064694/2/webrev/
>>
>> src/os/windows/vm/os_windows.cpp
>>     No commments.
>>
>> Thumbs up.
>>
>> Dan
>>
>>
>> On 11/18/14 12:29 AM, Ivan Gerasimov wrote:
>>> Hi Markus!
>>>
>>> The priority of the exiting thread will be raised for quite a short
>>> period of time -- right before the thread finishes exiting.
>>>
>>> There are two places where the priority is adjusted.
>>>
>>> Under normal conditions we should never see the first place hit.
>>> However, if we do, this means we have a huge number of threads.
>>> Raising the priority of one of them is a hint about which thread we
>>> want the scheduler to focus on.
>>>
>>> The second place is a bit different.
>>> We have several threads running immediately before ending the process.
>>> Some of them are at the exiting path and block exiting of the whole
>>> process.
>>> Raising the priority of those threads is a way to say we're not
>>> interested in all the other threads, as they are going to be
>>> terminated anyway.
>>>
>>> I just noticed that in second scenario it may be appropriate to set
>>> the priority of the current thread to the same level as for the
>>> exiting threads.
>>> This way it'll be given a fair chance to continue if the timeout
>>> expires.
>>>
>>> I also think it should be enough to set the priority level to
>>> THREAD_PRIORITY_ABOVE_NORMAL instead of THREAD_PRIORITY_HIGHEST.
>>> It will give just +1 to the priority value -- should be enough for
>>> the hint.
>>>
>>> Would you please take a look at the updated webrev:
>>> http://cr.openjdk.java.net/~igerasim/8064694/2/webrev/
>>>
>>> Sincerely yours,
>>> Ivan
>>>
>>>
>>> On 17.11.2014 11:33, Markus Grönlund wrote:
>>>> I agree with David.
>>>>
>>>> The side effects will be unknown and very hard to debug.
>>>>
>>>> Is there another way to accomplish the results without manipulating
>>>> base services?
>>>>
>>>> Thanks
>>>> Markus
>>>>
>>>> -----Original Message-----
>>>> From: David Holmes
>>>> Sent: den 17 november 2014 07:40
>>>> To: Ivan Gerasimov; Daniel Daugherty
>>>> Cc: serviceability-dev@openjdk.java.net; hotspot-runtime-dev
>>>> Subject: Re: RFR 8064694: Kitchensink: WaitForMultipleObjects failed
>>>> in hotspot\src\os\windows\vm\os_windows.cpp: 3844
>>>>
>>>> On 17/11/2014 7:23 AM, Ivan Gerasimov wrote:
>>>>> Thank you Daniel!
>>>>>
>>>>> Please find the updated webrev with your suggestions incorporated
>>>>> here:
>>>>> http://cr.openjdk.java.net/~igerasim/8064694/1/webrev/
>>>>>
>>>>> Concerning the thread priority: If the application is of
>>>>> NORMAL_PRIORITY_CLASS, then setting the thread's priority level to
>>>>> THREAD_PRIORITY_HIGHEST will result in its priority value to be only
>>>>> 10 (of maximum 31).
>>>>> http://msdn.microsoft.com/en-us/library/windows/desktop/ms685100(v=vs.
>>>>> 85).aspx
>>>>>
>>>>>
>>>>> And if the process is HIGH_PRIORITY_CLASS, then the tread with the
>>>>> HIGHEST priority level will have priority value == 15 of 31.
>>>>>
>>>>> I believe, it should not be too much, and the machine will not become
>>>>> busy with only those closing threads.
>>>>> However, I hope it would be enough to make them complete faster than
>>>>> other threads of the NORMAL priority level withing the same
>>>>> application.
>>>> I don't think this is necessary or desirable. Under normal usage
>>>> we're giving priority to exiting threads and that may disrupt the
>>>> usual scheduling patterns that applications see. You may posit that
>>>> it is "harmless" but we can't say that for sure. Nor can we actually
>>>> know that this will help with this particular bug. I would not add
>>>> in this new code.
>>>>
>>>> David
>>>>
>>>>> Sincerely yours,
>>>>> Ivan
>>>>>
>>>>>
>>>>> On 15.11.2014 2:22, Daniel D. Daugherty wrote:
>>>>>> On 11/14/14 5:35 AM, Ivan Gerasimov wrote:
>>>>>>> Hello!
>>>>>>>
>>>>>>> The recent fix for JDK-8059533 ((process) Make exiting process wait
>>>>>>> for exiting threads [win]) caused the warning message to be printed
>>>>>>> in some test environments:
>>>>>>> -----------
>>>>>>> os_windows.cpp:3844 is in the newly updated
>>>>>>> os::win32::exit_process_or_thread(Ept what, int exit_code)
>>>>>>> -----------
>>>>>>>
>>>>>>> This has been observed with debug builds on highly loaded systems.
>>>>>>>
>>>>>>>
>>>>>>> To address the issue it is proposed to do three things:
>>>>>>> 1) increase the timeout for debug builds,
>>>>>>> 2) increase the maximum number of the thread handles to be stored,
>>>>>>> 3) rise the priority of the exiting threads, if we need to wait for
>>>>>>> them.
>>>>>>>
>>>>>>> Would you please help review the fix?
>>>>>>>
>>>>>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8064694
>>>>>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8064694/0/webrev/
>>>>>> src/os/windows/vm/os_windows.cpp
>>>>>>
>>>>>>    line 3784: #define MAX_EXIT_HANDLES NOT_DEBUG(32) DEBUG_ONLY(128)
>>>>>>      Instead of NOT_DEBUG can you use PRODUCT_ONLY?
>>>>>>      Instead of DEBUG_ONLY can you used NOT_PRODUCT?
>>>>>>
>>>>>>      That uses the smaller value for only one build config (PRODUCT).
>>>>>>
>>>>>>    line 3785: #define EXIT_TIMEOUT     NOT_DEBUG(1000)
>>>>>> DEBUG_ONLY(4000)
>>>>>> /*1 sec in product, 4 sec in debug*/
>>>>>>      Instead of NOT_DEBUG can you use PRODUCT_ONLY?
>>>>>>      Instead of DEBUG_ONLY can you used NOT_PRODUCT?
>>>>>>      Please add spaces between the comment delimiters and the comment
>>>>>> text.
>>>>>>
>>>>>>      That uses the smaller timeout for only one build config
>>>>>> (PRODUCT).
>>>>>>
>>>>>>    line 3836           // Rise the priority...
>>>>>>      Typo: 'Rise' -> 'Raise'
>>>>>>
>>>>>>      About the general idea of raising the exiting thread's priority,
>>>>>>      if the exiting thread is looping in some Win* OS code after this
>>>>>>      point, will raising the priority make the machine unusable?
>>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>>> The fix was tested on all available platforms, with the hotspot
>>>>>>> testset. No failures.
>>>>>>>
>>>>>>> Sincerely yours,
>>>>>>> Ivan
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>
>>
>>
>>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic