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

Thu Feb 28 19:01:23 UTC 2019

Hi Zhengyu,

On Thu, Feb 28, 2019 at 3:26 PM <zgu at redhat.com> wrote:

> On Wed, 2019-02-27 at 16:31 +0100, Thomas Stüfe wrote:
> Hi Thomas,
>
> > Hi Zhengyu,
> >
> > First thanks alot for tackling this! I understand this was in large
> > parts to make AIX and us happy, which we are thankful for (as should
> > be IBM :).
>
> Not really.
>
> It was not pleasant (really somewhat embarrassing) when I had to
> explain why some of numbers don't add up or even make sense (e.g. total
> committed > RSS).  Now, you have thread stacks in summary, but totally
> absent in details, it is quite noticeable, isn't it?
>
> I don't have to answer to any curious minds so far (not sure about
> you), but not so sure about that in near future :-)
>

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?

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

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

--

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?

--

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?

Cheers, Thomas

>
> Thanks,
>
> -Zhengyu
>
>
>
>
> >
> > Thanks Thomas
> >
> >
> >
> > On Mon, Feb 25, 2019 at 6:50 PM <zgu at redhat.com> wrote:
> > > On Mon, 2019-02-25 at 18:21 +0100, Thomas Stüfe wrote:
> > > > Short question, just to clarify things:
> > > >
> > > > when you talk about "virtual memory thread stack" and "malloced
> > > > thread stack" you mean actually
> > > > a) a thread stack whose boundaries are page aligned and which can
> > > > have uncommitted portions and we are able to find that out (e.g.
> > > > mincore)
> > > > b) a thread stack whose boundaries are not necessarily page
> > > aligned
> > > > and which either have no uncommitted portions or we are unable to
> > > > find that out?
> > > >
> > > > And the whole "malloc" terminology just stems from the fact that
> > > you
> > > > use the malloc tracking mechanics of NMT to track the latter,
> > > just
> > > > because it is a good fit?
> > > No, I do mean if the thread stack is backed by virtual memory or
> > > malloc'd memory.
> > >
> > > The thread stack is backed by malloc'd memory, it does not expect
> > > to
> > > have guard pages. If it is not the case, then this patch is still
> > > broken.
> > >
> > > Thanks,
> > >
> > > -Zhengyu
> > >
> > >
> > > >
> > > > ..Thomas
> > > >
> > > >
> > > > On Mon, Feb 25, 2019 at 5:10 PM <zgu at redhat.com> wrote:
> > > > > Friendly ping!
> > > > >
> > > > > Thanks,
> > > > >
> > > > > -Zhengyu
> > > > >
> > > > > On Mon, 2019-02-18 at 14:57 -0500, zgu at redhat.com wrote:
> > > > > > On Mon, 2019-02-18 at 08:50 -0500, zgu at redhat.com wrote:
> > > > > > > NMT tracks thread stacks as virtual memory since day one,
> > > as
> > > > > most
> > > > > > > of
> > > > > > > platofrms are using virtual memory to back thread stacks.
> > > > > However,
> > > > > > > there are exceptions, e.g. AIX. The matter of fact, POSIX
> > > > > standard
> > > > > > > does
> > > > > > > not dictate what kind of memory or even alignment
> > > requirement
> > > > > for
> > > > > > > thread stacks.
> > > > > > >
> > > > > > > This patch separates thread stack tracking from virtual
> > > memory
> > > > > > > tracking. It delegates to virtual memory tracker if thread
> > > > > stack is
> > > > > > > backed by virtual memory.
> > > > > > >
> > > > > > > For thread stacks that are backed by malloc'd memory, it
> > > can
> > > > > not
> > > > > > > simply
> > > > > > > utilize malloc tracker for detail tracking, since it can
> > > not
> > > > > > > install
> > > > > > > tracking header in place. Therefore, it has to track those
> > > > > memory
> > > > > > > separately, then piggybacks to existing malloc tracking
> > > > > > > infrastructure
> > > > > > > during snapshot, for baselining and reporting.
> > > > > > >
> > > > > > > Thanks for Goetz Lindenmaier and Thomas Stüfe's help to run
> > > the
> > > > > > > patch
> > > > > > > through SAP buidling and testing infrastructure, that is
> > > > > especially
> > > > > > > helpful for verifying the fix for AIX.
> > > > > > >
> > > > > > >
> > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8204552
> > > > > > > Webrev: http://cr.openjdk.java.net/~zgu/JDK-8204552/webrev.
> > > 01/
> > > > > > >
> > > > > > > Test:
> > > > > > >   hotspot_nmt on Linux x64 (fastdebug and release)
> > > > > >
> > > > > > Also passed JDK-submit tests.
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > -Zhengyu
> > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > -Zhengyu
> > > > > > >
> > > > > > >
> > > > > > >
>