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

[Zhengyu Gu]

Thanks for the review, Coleen.

I am currently on vacation, will push after return.

-Zhengyu 

Sent from my iPad

> On Apr 18, 2018, at 6:30 AM, coleen.phillimore at oracle.com wrote:
> 
> 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
>>>>> 
>