RFR 8204552: NMT: Separate thread stack tracking from virtual memory tracking

Thu Mar 7 16:10:52 UTC 2019

Hi Zhengyu,

thanks for the changes. I compiled and tested on AIX.

See here AIX output for a small test creating 100 child processes and
waiting on them, which creates 100 reaper threads with small thread stacks
(128K by default but AIX has a high lower cap, reaper threads have ~450K
stack bc of 64K pages). Baseline is about 50M stack.

(Note that this is not committed thru but NMT does not show this with this
mode).

https://gist.github.com/tstuefe/734c8c0bb8987f5ff8fd1ca2155bfc17

I think this checks out.

---

Here, I have a test output of our own propietary malloc statistics:

https://gist.github.com/tstuefe/860b5a1d2500ab79df67254879a1d30f#file-sapjvm-malloc-statistics-sample

Showing you this to maybe get a feeling how memory consumption would be if
we would track every malloc (which I think we should). This is for a test
running about 2 hours. We instrument everything, no lower threshold. As you
see, we have about ~320 callsites including those few from JDK (we have
instrumented JDK too). However we do not track callstacks so most of these
callsites may explode to multiple callstacks for you in NMT. However
however, since you forfeit only low memory callsites, I expect that effect
not to be large.

----

following remarks inline:

On Mon, Mar 4, 2019 at 10:05 PM <zgu at redhat.com> wrote:

> On Thu, 2019-02-28 at 20:01 +0100, Thomas Stüfe wrote:
> >
> Hi Thomas,
>
> > Ah, I get it now. Yes, I know the type, some folks are just way too
> > curious :)
> >
> > On the other hand, cross checking statistical numbers and requiring
> > them to make sense makes, well, sense. If they do, it proves you have
> > understood the issue and that your statistics are reliable.
> >
> > > >
> > > > That said, lets back up a bit, because I still do not understand
> > > what
> > > > you do:
> > > >
> > > > Not sure if I claimed at some point AIX clib allocates thread
> > > stacks
> > > > with malloc? If yes, this was a misunderstanding. I actually do
> > > not
> > > > know which API they use to allocate stacks (aix libc is closed
> > > > source) but I know the effects: thread stack is located in the
> > > data
> > > > segment and not aligned. So it could be allocated with malloc,
> > > but
> > > > also with something like sbrk(). Not sure if this is even
> > > important.
> > >
> > > No, it is not important, but it has to go somewhere. If it is not
> > > malloc, then virtual memory with unaligned address? and possibly
> > > unaligned size? it invites more skepticism.
> >
> > Okay, you mean NMT tracks now VM and malloc, and we need to account
> > for thread stacks somewhere, so we need to use one of the existing
> > mechanisms?
>
> The answer is no, but probably not worth to invent another one, what do
> you think? Besides NMT always special cases mtThreadStack since
> beginning.
>
> I agree. Lets not make this unnecessray complicated.

> >
> > >
> > > >
> > > > I would rather go with some form of "attribute based"
> > > implementation.
> > > > Rather than have one implementation for vm, one for malloc, we
> > > could
> > > > have one which assumes stack boundaries are page aligned and
> > > pages
> > > > can be uncommitted; one where we only track the size and nothing
> > > has
> > > > to be aligned. Basically the same you do in your patch, but I
> > > would
> > > > not name it "..malloc.."
> > >
> > > Okay, I am fine with not naming it "malloc" during reporting.
> > > What's
> > > your suggestion?
> >
> > It is not so much reporting I am concerned with but the code itself.
> > I can live with the numbers for threads to appear as "malloced" to
> > the customer, but inside the code - for maintainers and possible
> > future porters of your code - I would like to be more exact with the
> > terminology.
> >
> > Note that "not aligned page boundaries and cannot call mincore on it"
> > could apply to other forms of stack management as well. For instance,
> > a hypothetical VM implementation where the stacks are managed by user
> > code itself (you can start a thread and provide your own stack).
> >
> > Proposal: I would just rename "MallocThreadStack" to
> > "SimpleThreadStack" or similar, meaning that no assumptions are made
> > about the alignment of the stack boundaries and that we do not know
> > how to find out the size of the committed space. This would be a
> > minimal implementation for handicapped platforms like AIX. The
> > implementation could remain the same.
>
> Done. I also remoevd "malloc" from thread stack reporting for
> !track_as_vm() cases.
>
>
thank you!

> e.g.
>
> -                    Thread (reserved=30937KB, committed=30937KB)
>                             (thread #30)
>                             (Stack: 30784KB)
>                             (malloc=115KB #182)
>                             (arena=38KB #58)
>
> >
> > > >
> > > > However, I am not even sure if we need all that machinery just to
> > > > track stacks allocation sites. Is that so exciting?
> > > Again, no, but it has to come from somewhere, right?
> > >
> > > > I would assume to see always the same stacks, since there are not
> > > > many variations of "start java thread". Sure it is cool to see.
> > > But
> > > > to me a summary number "sum of java stack memory" would be
> > > > sufficient, maybe in Threads beside the java thread counter. One
> > > > advantage this would have is that we do not need NMT to be on to
> > > > track that.
> > > Again, my intention is to make numbers adding up as closely as
> > > possible.
> >
> > I understood that now. Thank you for your patience.
> >
> > So now that I know the background, I took a closer look at your
> > patch. Beside my general concern - that I do not like the "malloc"
> > naming in all the code - there are few remarks. Seems all solid:
> >
> > src/hotspot/share/services/memBaseline.cpp
> >
> > Question: You use MallocAllocationSiteWalker to walk the thread
> > stacks. But that will exclude allocations considered too small, <
> > SIZE_THRESHOLD. Would this not confuse your numbers?
> >
> Yep. This is indeed a source of inaccuracy :-( IIRC, it was due to the
> concern of tracking memory overhead, probably unfounded.
>
>
See above. I do not think it would be that costly.

> > --
> >
> > src/hotspot/share/services/memReporter.cpp
> >
> > Of course, here we have exactly the problem you described, that we
> > report more than RSS since thread stacks are not completely committed
> > on AIX. But I can live with that.
> >
> > Looks good. I assume you tried out all that reporting code by
> > "faking" malloc-like thread stack allocation on your machine?
> Yes, had to disable a few asserts.
>
> > --
> >
> > src/hotspot/share/services/threadStackTracker.cpp, hpp: looks ok.
> > Should we maybe assert that when deleting a thread stack
> > (ThreadStackTracker::delete_thread_stack), stack size was the same as
> > with create?
> >
> Added assertion in SimpleThreadStackSite::equals(), okay?
>
> Updated webrev: http://cr.openjdk.java.net/~zgu/JDK-8204552/webrev.02/
>
> Thanks,
>
> -Zhengyu
>
> >
>

webrev03 looks ok to me.

Thanks!

..Thomas

>
>