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

Tue Nov 18 13:02:45 UTC 2014

Hi Ivan,

I don't want to you block you from getting this in - I need to get the full story behind all these changes (backtracking now).

If I find something that I think we should revisit, we can always do that later.

So pls go ahead.

Thanks
Markus

PS.
I have some concerns (but will need to get back to you on that after tracing down the exact details)).

Do you have a particular test case that you have been working on for these changes?

-----Original Message-----
From: Ivan Gerasimov 
Sent: den 18 november 2014 08:30
To: Markus Grönlund; David Holmes; 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

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