RFR(XS) 8191369: NMT: Enhance thread stack tracking

[Zhengyu Gu]

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