RFR(XS) 8191369: NMT: Enhance thread stack tracking
Zhengyu Gu
zgu at redhat.com
Fri Feb 2 17:13:40 UTC 2018
PING!
Thanks,
-Zhengyu
On 11/17/2017 03:43 PM, Zhengyu Gu wrote:
> There were two bugs that resulted NMT report incorrect thread stacks.
>
> 1) NMT did not handle overlapped regions correctly.
>
> 2) There are two issues with Linux: get_stack_commited_bottom()
>
> * The method name is misleading, it does not return *committed*
> bottom, but *mapped* bottom, where address space is valid, not necessary
> committed.
> I renamed this function to get_stack_mapped_bottom() with option to
> return committed bottom with proper parameter.
>
> * The binary search algorithm mishandled edge cases.
>
> The test case for verifying fixes for above two issues can be found
> here: http://cr.openjdk.java.net/~zgu/8191369/test_mmap.c
>
>
> As mentioned in early email, new patch also contains implementation for
> Windows.
>
> Updated Webrev: http://cr.openjdk.java.net/~zgu/8191369/webrev.01/
>
> Test:
>
> Reran hotspot_tier1 tests on Windows x64 and Linux x64 (fastdebug and
> release)
>
> Thanks,
>
> -Zhengyu
>
>
> On 11/16/2017 03:42 PM, Zhengyu Gu wrote:
>> Sorry, I have to withdraw this code review request, since the stack
>> sizes reported on Linux do not match /proc/<pid>/smaps.
>>
>> I am debugging the problem, will submit new request later.
>>
>> Thanks,
>>
>> -Zhengyu
>>
>> On 11/16/2017 01:30 PM, Zhengyu Gu wrote:
>>> Hi,
>>>
>>> I added Windows support and updated bug to make this enhancement
>>> Linux and Windows only.
>>>
>>> As Thomas suggested, other platforms can be addressed by followup fixes.
>>>
>>> Updated Webrev: http://cr.openjdk.java.net/~zgu/8191369/webrev.01/
>>>
>>> I reran hotspot_tier1_runtime on Linux x64 and Windows 10 (fastdebug
>>> + release)
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>>
>>>
>>> On 11/16/2017 04:45 AM, Thomas Stüfe wrote:
>>>>
>>>>
>>>> On Thu, Nov 16, 2017 at 10:23 AM, Severin Gehwolf
>>>> <sgehwolf at redhat.com <mailto:sgehwolf at redhat.com>> wrote:
>>>>
>>>> On Thu, 2017-11-16 at 08:14 +0100, Thomas Stüfe wrote:
>>>> > On Thu, Nov 16, 2017 at 7:58 AM, David Holmes
>>>> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>>
>>>> > wrote:
>>>> >
>>>> > > Hi Zhengyu,
>>>> > >
>>>> > > I can't review this in detail as I'm not familiar with NMT
>>>> workings, but
>>>> > > have a couple of comments.
>>>> > >
>>>> > > On 16/11/2017 8:25 AM, Zhengyu Gu wrote:
>>>> > >
>>>> > > > This is Linux only enhancement for now, can be extended
>>>> for other
>>>> > > > platforms. (See bug for details)
>>>> > > >
>>>> > >
>>>> > > I'm concerned about unnecessary platform skew. I added some
>>>> comments to
>>>> > > the bug. I think the mincore trick can be used on Solaris
>>>> and AIX as well -
>>>> > > but not on OS X / BSD. It may be that VirtualQuery can be
>>>> used on Windows
>>>> > > to get the same information - but I'm unclear if the memory
>>>> states support
>>>> > > what you're looking for. It would be good to extend this to
>>>> other platforms
>>>> > > that will support it.
>>>> > >
>>>> >
>>>> > Just 5c:
>>>> >
>>>> > I think this is definitely useful even with only the Linux
>>>> implementation
>>>> > provided.
>>>>
>>>> +1
>>>>
>>>> While plaform skew is a valid concern, I'm doubtful it's
>>>> "unnecessary".
>>>> I think there is value to get one platform fixed and then look
>>>> at other
>>>> platforms one by one as follow-up bugs. For some platforms it
>>>> might be
>>>> hard to do (or undesirable) as Thomas says.
>>>>
>>>> Thanks,
>>>> Severin
>>>>
>>>>
>>>> I volunteer to implement this for windows if it can be done as
>>>> non-time-critical follow up, if no-one else has stepped up in until
>>>> then.
>>>>
>>>> ..Thomas
>>>>
>>>>
>>>> > On AIX we have commit-on-touch and hence mincore may actually
>>>> force commit
>>>> > the thread :P - would have to try it but have no high hopes.
>>>> Well.
>>>> >
>>>> > On Windows this is possible, we already use VirtualQuery in our
>>>> fork to
>>>> > print out the commit-state of the current thread stack in
>>>> case of
>>>> an error.
>>>> >
>>>> > ..Thomas
>>>> >
>>>> >
>>>> > > And on a minor note can you please correct the existing
>>>> spelling error in
>>>> > > get_stack_commited_bottom. :)
>>>> > >
>>>> > > Current implementation, thread stack is tracked when thread is
>>>> created,
>>>> > > > its available stack space is tagged as reserved and
>>>> committed.
>>>> > > >
>>>> > > > However, this is not how stack works. Stack pages are
>>>> committed on
>>>> > > > demand, so this approach overstates its memory usage.
>>>> > > >
>>>> > > > This enhancement gathers thread stack usage at NMT query
>>>> time, so that it
>>>> > > > can report more accurate usage.
>>>> > > >
>>>> > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8191369
>>>> <https://bugs.openjdk.java.net/browse/JDK-8191369>
>>>> > > > Webrev:
>>>> http://cr.openjdk.java.net/~zgu/8191369/webrev.00/index.html
>>>> <http://cr.openjdk.java.net/~zgu/8191369/webrev.00/index.html>
>>>> > > >
>>>> > > >
>>>> > > > Example:
>>>> > > >
>>>> > > > Summary shows thread stacks only use small faction of
>>>> reserved space.
>>>> > > >
>>>> > > > - Thread (reserved=41216KB,
>>>> committed=632KB)
>>>> > > > (thread #41)
>>>> > > > (stack: reserved=41032KB,
>>>> committed=448KB)
>>>> > > > (malloc=137KB #222)
>>>> > > > (arena=47KB #76)
>>>> > > >
>>>> > > >
>>>> > > > Detail tracking shows stack guards and actually
>>>> used/committed stack area.
>>>> > > >
>>>> > > > [0x00007f66e95e7000 - 0x00007f66e96e8000] reserved 1028KB
>>>> for
>>>> Thread
>>>> > > > Stack from
>>>> > > > [0x00007f67c0b57d5c] JavaThread::run()+0x2c
>>>> > > > [0x00007f67c08bbea2] thread_native_entry(Thread*)+0x112
>>>> > > >
>>>> > > > [0x00007f66e95e7000 - 0x00007f66e95eb000] committed
>>>> 16KB from
>>>> > > > [0x00007f67c08b7319]
>>>> os::pd_create_stack_guard_pages(char*,
>>>> > > > unsigned long)+0x29
>>>> > > > [0x00007f67c0b56851]
>>>> JavaThread::create_stack_guard_pages()
>>>> > > > [clone .part.125]+0xa1
>>>> > > > [0x00007f67c0b57d6e] JavaThread::run()+0x3e
>>>> > > > [0x00007f67c08bbea2]
>>>> thread_native_entry(Thread*)+0x112
>>>> > > >
>>>> > > > [0x00007f66e96e7000 - 0x00007f66e96e8000]
>>>> committed 4KB
>>>> > > >
>>>> > >
>>>> > > Can we capture this in a test so we can tell that the
>>>> tracking has
>>>> > > improved?
>>>> > >
>>>> > > Thanks,
>>>> > > David
>>>> > > -----
>>>> > >
>>>> > > Thanks,
>>>> > > >
>>>> > > > -Zhengyu
>>>> > > >
>>>> > > >
>>>>
>>>>
More information about the hotspot-runtime-dev
mailing list