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

zgu at redhat.com zgu at redhat.com
Mon Mar 4 21:05:54 UTC 2019 

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

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.

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

> 
> 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/web
> > rev.
> > > > 01/
> > > > > > > > 
> > > > > > > > Test:
> > > > > > > >   hotspot_nmt on Linux x64 (fastdebug and release)
> > > > > > > 
> > > > > > > Also passed JDK-submit tests.
> > > > > > > 
> > > > > > > Thanks,
> > > > > > > 
> > > > > > > -Zhengyu
> > > > > > > 
> > > > > > > > 
> > > > > > > > Thanks,
> > > > > > > > 
> > > > > > > > -Zhengyu
> > > > > > > > 
> > > > > > > > 
> > > > > > > >

More information about the hotspot-runtime-dev mailing list