llvm dev - Apr 2014 - [LLVMdev] [RFC] [PATCH] Fix for sys::Process::GetMallocUsage() when using ptmalloc2 allocator in glibc

Thanks for the reply.

On 28 April 2014 20:01, Reid Kleckner <rnk at google.com>
wrote:
> The problem with tcmalloc seems like a real problem.  I can't think of
any
> good workarounds.  My best worst idea is to try to figure out if malloc is
> coming from libc with dlsym and dlopen, and then use that to decide whether
> we add these two numbers together.
I'm not sure we can always assume libc is a dynamic library. A user
can statically link to libc if they really want to. For the static
case something like libbfd could help provided the symbol table was
still around.

It seems to me whatever we do is going to incur some overhead. Calling
dlsym() sounds a little bit expensive so we might be best off doing
the test on the first call to sys::Process::GetMallocUsage() and
storing a static variable that indicates the result of the test so we
don't need to do it again on subsequent calls to GetMallocUsage().
This would mean if an application decided to change its allocator
during execution GetMallocUsage() might break but changing the
allocator during execution sounds insane anyway, so hopefully no one
does that.

-- 
Dan Liew
PhD Student - Imperial College London

On Mon, Apr 28, 2014 at 3:40 PM, Dan Liew <dan at su-root.co.uk> wrote:
> Thanks for the reply.
>
> On 28 April 2014 20:01, Reid Kleckner <rnk at google.com> wrote:
> > The problem with tcmalloc seems like a real problem.  I can't
think of
> any
> > good workarounds.  My best worst idea is to try to figure out if
malloc
> is
> > coming from libc with dlsym and dlopen, and then use that to decide
> whether
> > we add these two numbers together.
>
> I'm not sure we can always assume libc is a dynamic library. A user
> can statically link to libc if they really want to. For the static
> case something like libbfd could help provided the symbol table was
> still around.
>
> It seems to me whatever we do is going to incur some overhead. Calling
> dlsym() sounds a little bit expensive so we might be best off doing
> the test on the first call to sys::Process::GetMallocUsage() and
> storing a static variable that indicates the result of the test so we
> don't need to do it again on subsequent calls to GetMallocUsage().
> This would mean if an application decided to change its allocator
> during execution GetMallocUsage() might break but changing the
> allocator during execution sounds insane anyway, so hopefully no one
> does that.

I think the static libc case breaks the dlsym trick.  The sanitizers, for
example, provide a statically linked malloc implementation.

It's probably OK to have one-time startup overhead.  We only use this for
-time-passes.  So... allocate 10mb and see how the counters update?  =D
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140428/22b3a944/attachment.html>

> It's probably OK to have one-time startup overhead.  We only use this
for
> -time-passes.  So... allocate 10mb and see how the counters update?  =D
That is an option but that itself has problems...

- The test would be dependent on the threshold used by the allocator
to decided whether or not to use mmap(). If we pick a too large amount
of memory to allocate users might get annoyed. If we pick too small
amount of memory to allocate then changes to the allocator might break
the test.

- Thread safety. In a multi-threaded application there is a small
chance that additional mallocs (other than our own) may happen between
the first read from mallinfo() and the second read. If those
additional mallocs internally use mmap() there's no problem
(mallinfo.uordblks won't increase) but if they use sbrk()
mallinfo.uordblks will increase and possibly invalidate the test.
Maybe this isn't worth worrying about...

Attached is a rough draft of what you are suggesting. 10MiB wasn't
enough on my system to trigger mmap() usage but 20MiB was (the flaws
in our approach are already showing!). I wouldn't suggest you commit
this but feel free to try out the patch to see how it performs. It
"works fine for me" but I have no idea about other systems.

We probably ought to get the glibc devs to try and fix this...

-- 
Dan Liew
PhD Student - Imperial College London
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Try-to-fix-llvm-sys-Process-GetMallocUsage-under-all.patch
Type: text/x-patch
Size: 3864 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140429/24f8c1cb/attachment.bin>