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

[Thomas Stüfe]

On Thu, Nov 16, 2017 at 10:23 AM, Severin Gehwolf <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>
> > 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
> > > > Webrev: 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
> > > >
> > > >
>
>