RFR: 8199067 [REDO] NMT: Enhance thread stack tracking

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Tue Apr 17 22:30:42 UTC 2018


Hi Zhengyu,
I'm sorry for the delay.  I did look at this and it seems fine. Thank 
you for adding the test for it.
thanks,
Coleen

On 4/12/18 4:13 PM, Zhengyu Gu wrote:
> Hi Thomas,
>
> Thanks for the review.
>
> On 04/11/2018 10:38 AM, Thomas Stüfe wrote:
>> Hi Zhengyu,
>>
>> sorry for the late response!
>>
>> I looked the windows changes over.
>>
>> The capping of the size may not work correctly if the committed range
>> overlaps the end of the region but starts in the middle:
>>
>> +    // current region may go beyond the limit, trim to the limit
>> +    committed_size = MIN2(committed_size, size);
>>
>> should it not be rather:
>>
>> +    committed_size = MIN2(committed_size, top - committed_start);
>>
>> ?
>>
>> The code also assumes that the input range starts at the begin of a
>> region. Not sure if this is always the case. So, I would test if
>> MEMORY_BASIC_INFORMATION.BaseAddress lays below the input start
>> address, in which case I would only add the overlapping part to the
>> committed size.
>
> My bad. When I converted the method from thread stack specific one to 
> this general one, I did not think through and make corresponding 
> changes. Thanks for catching this.
>
> I renamed gtest test to reflect that it is no longer thread stack 
> specific, and added test cases for it.
>
> Updated webrev: 
> http://cr.openjdk.java.net/~zgu/8199067/webrev.01/index.html
>
> Test:
>   hotspot_runtime and gtest on Linux x64 (fastdebug and release)
>                                Windows x64 (fastdebug and release)
>
> Thanks,
>
> -Zhengyu
>
>
>
>
>
>>
>> Best Regards, Thomas
>>
>>
>>
>>
>>
>>
>> On Mon, Mar 19, 2018 at 5:10 PM, Zhengyu Gu <zgu at redhat.com> wrote:
>>>
>>> Ping!
>>>
>>> submit-hs test still good.
>>>
>>> Mach5 mach5-one-zgu-JDK-8199067-20180319-1419-14981: Builds PASSED. 
>>> Testing SUCCESSFUL.
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>>
>>> On 03/08/2018 09:08 AM, Zhengyu Gu wrote:
>>>>
>>>> The early change failed on some fast machine with many processors 
>>>> running Linux OS. It reveals that there can be holes in thread's 
>>>> stack, which throws off binary search for finding stack bottom.
>>>>
>>>> Besides, some threads (ConcurrentGCThread and WatcherThread, etc.) 
>>>> do exit without notifying NMT, which can trip over NMT final 
>>>> reporting, as their stacks became unmapped.
>>>>
>>>> This patch addresses above issues by searching whole address range 
>>>> for committed regions. If the address range becomes invalid, simply 
>>>> reports as not committed.
>>>>
>>>> GTest also updated.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8199067
>>>> Webrev: http://cr.openjdk.java.net/~zgu/8199067/webrev.00/index.html
>>>>
>>>>
>>>> Test:
>>>>     hotspot tier1 on Linux 64 and Windows x64.
>>>>
>>>>     Submit repo test came back clean.
>>>>     Mach5 mach5-one-zgu-JDK-8199067-20180308-1218-13719: Builds 
>>>> PASSED. Testing SUCCESSFUL.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>>



More information about the hotspot-runtime-dev mailing list