RFR 8064694: Kitchensink: WaitForMultipleObjects failed in hotspot\src\os\windows\vm\os_windows.cpp: 3844

Tue Nov 18 15:27:36 UTC 2014

 > 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 at 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
>>>>>
>>>>
>>>>
>>
>