Re: Do we still need Trace Malloc?

2014-05-21 Thread Nicholas Nethercote
On Mon, May 19, 2014 at 5:32 PM, L. David Baron  wrote:
>
> There are some things that I do with trace-malloc that I'm not sure
> if the other tools do.

I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1014341 for
removing trace-malloc. Please add any dependencies that I've missed.
Thanks!

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Do we still need Trace Malloc?

2014-05-20 Thread Ehsan Akhgari

On 2014-05-19, 10:25 AM, Nicholas Nethercote wrote:

It's used to get stacks within the deadlock detector, but I'm not sure
if that's necessary, and it doesn't seem like it would be that hard to
replace if it is necessary.


You would think so, but I tried in bug 939231 and failed.

Cheers,
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Do we still need Trace Malloc?

2014-05-20 Thread Benoit Jacob
2014-05-19 23:19 GMT-04:00 L. David Baron :

> On Monday 2014-05-19 20:09 -0700, Nicholas Nethercote wrote:
> > On Mon, May 19, 2014 at 5:32 PM, L. David Baron 
> wrote:
> > > Another is being able to find the root strongly connected components
> > > of the memory graph, which is useful for finding leaks in other
> > > systems (e.g., leaks of trees of GTK widget objects) that aren't
> > > hooked up to cycle collection.  It's occasionally even a faster way
> > > of debugging non-CC but nsTraceRefcnt-logged reference counted
> > > objects.
> >
> > How does trace-malloc do that? It sounds like it would need to know
> > about object and struct layout.
>
> Roughly the same way a conservative collector would -- assuming any
> word-aligned memory in one object in the heap that contains
> something that's the address of something else in the heap
> (including in the interior of the allocation) is a pointer to that
> object in the heap.
>
> (It's actually done in the leaksoup tool outside of trace-malloc.)
>

For that, I believe that the right approach at this point would be to use
DMD's memory/replace tool (maybe evolving it to suit your needs),

http://hg.mozilla.org/mozilla-central/file/cb9f34f73ebe/memory/replace/dmd

You could also write your own memory/replace tool sitting next to that one,
but it seems that every such tool needs to do roughly the same things, i.e.
store metadata around heap blocks and allow iterating over them, so they
might as well be the same tool.

Notice that I needed to do the same things in refgraph (
https://github.com/bjacob/mozilla-central/wiki/Refgraph, a tool to
investigate the graph of strong references between heap blocks) and at that
time wrote my own memory/replace tool (memory/replace/refgraph in that
fork). But it's been the most expensive part of that fork, in terms of
maintenability and portability. If I had time to continue work on this, the
first thing I'd do would be to drop that custom memory/replace tool and
instead just use DMD's.

Benoit


>
> -David
>
> --
> 𝄞   L. David Baron http://dbaron.org/   𝄂
> 𝄢   Mozilla  https://www.mozilla.org/   𝄂
>  Before I built a wall I'd ask to know
>  What I was walling in or walling out,
>  And to whom I was like to give offense.
>- Robert Frost, Mending Wall (1914)
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Do we still need Trace Malloc?

2014-05-19 Thread L. David Baron
On Monday 2014-05-19 20:09 -0700, Nicholas Nethercote wrote:
> On Mon, May 19, 2014 at 5:32 PM, L. David Baron  wrote:
> > Another is being able to find the root strongly connected components
> > of the memory graph, which is useful for finding leaks in other
> > systems (e.g., leaks of trees of GTK widget objects) that aren't
> > hooked up to cycle collection.  It's occasionally even a faster way
> > of debugging non-CC but nsTraceRefcnt-logged reference counted
> > objects.
> 
> How does trace-malloc do that? It sounds like it would need to know
> about object and struct layout.

Roughly the same way a conservative collector would -- assuming any
word-aligned memory in one object in the heap that contains
something that's the address of something else in the heap
(including in the interior of the allocation) is a pointer to that
object in the heap.

(It's actually done in the leaksoup tool outside of trace-malloc.)

-David

-- 
𝄞   L. David Baron http://dbaron.org/   𝄂
𝄢   Mozilla  https://www.mozilla.org/   𝄂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Do we still need Trace Malloc?

2014-05-19 Thread Nicholas Nethercote
On Mon, May 19, 2014 at 5:32 PM, L. David Baron  wrote:
>
> There are some things that I do with trace-malloc that I'm not sure
> if the other tools do.
>
> One is to check for leaks that involve caches (i.e., don't involve
> unreachable pointers).  One can take two memory dumps at different
> times and build a allocation-stack-tree-diff of the two dumps.

It should be easy to modify DMD to emulate this.

> Another is being able to find the root strongly connected components
> of the memory graph, which is useful for finding leaks in other
> systems (e.g., leaks of trees of GTK widget objects) that aren't
> hooked up to cycle collection.  It's occasionally even a faster way
> of debugging non-CC but nsTraceRefcnt-logged reference counted
> objects.

How does trace-malloc do that? It sounds like it would need to know
about object and struct layout.

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Do we still need Trace Malloc?

2014-05-19 Thread Mike Hommey
On Mon, May 19, 2014 at 05:07:44PM -0700, Nicholas Nethercote wrote:
> On Mon, May 19, 2014 at 3:05 PM, L. David Baron  wrote:
> >> Do we still need Trace Malloc?
> >
> > Are you talking about removing it from the debug builds done on our
> > infra, or removing it from the tree?
> 
> I'm aiming for the latter, though the former is a reasonable first step :)
> 
> > I think the former is fine;
> > I'd like to know more about the memory graph analysis abilities of
> > ASAN before being ok with the latter.
> >
> > (It was enabled because we used to track shutdown leak metrics and
> > some other metrics, but when we switched to tracking
> > performance/memory metrics from graphs to tracking them via
> > regression notices, regression notices weren't added for those
> > measurements, so they just kept regressing without people noticing.
> > At some point we then stopped running them because nobody was
> > looking at them.  I wouldn't mind having that metric tracked again
> > in some way because it does catch some real leak regressions in
> > addition to shutdown leaks.)
> 
> Andrew McCreight is working on getting LSAN enabled by default on TBPL
> (bug 976414). He's already found a bunch of leaks using it; see the
> blocking bugs. LSAN's docs are here:
> https://code.google.com/p/address-sanitizer/wiki/LeakSanitizer.
> There's not much info on how it works, though I suspect it's extremely
> similar to how Valgrind's leak checking works, which is described in
> some detail here:
> http://www.valgrind.org/docs/manual/mc-manual.html#mc-manual.leaks
> 
> If you're still concerned, porting this feature of Trace Malloc to DMD
> is an option. But in general, tools like Valgrind and ASAN/LSAN do a
> better job of leak detection than simple malloc replacement tools like
> Trace Malloc, because they are able to do a conservative scan of
> addressable memory to find pointers. This means they can distinguish
> truly unreachable blocks from blocks that are still reachable but we
> didn't bother freeing. In practice, the number of blocks in the latter
> category is huge, and IME tools that can't distinguish the two cases
> are very difficult to use.

OTOH, *SAN tools are only running on Linux, and thus won't find any
leaks that come from platform-dependent code.

Mike
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Do we still need Trace Malloc?

2014-05-19 Thread L. David Baron
On Monday 2014-05-19 17:07 -0700, Nicholas Nethercote wrote:
> On Mon, May 19, 2014 at 3:05 PM, L. David Baron  wrote:
> >> Do we still need Trace Malloc?
> >
> > Are you talking about removing it from the debug builds done on our
> > infra, or removing it from the tree?
> 
> I'm aiming for the latter, though the former is a reasonable first step :)
> 
> > I think the former is fine;
> > I'd like to know more about the memory graph analysis abilities of
> > ASAN before being ok with the latter.

This point was separate from the following one.

There are some things that I do with trace-malloc that I'm not sure
if the other tools do.

One is to check for leaks that involve caches (i.e., don't involve
unreachable pointers).  One can take two memory dumps at different
times and build a allocation-stack-tree-diff of the two dumps.  Thus
one can get into a certain state, do things that should lead to the
memory usage being the same as the first state, and see if anything
interesting increased (with allocation stacks).  This has been
useful for finding caches that weren't being cleared properly.

Another is being able to find the root strongly connected components
of the memory graph, which is useful for finding leaks in other
systems (e.g., leaks of trees of GTK widget objects) that aren't
hooked up to cycle collection.  It's occasionally even a faster way
of debugging non-CC but nsTraceRefcnt-logged reference counted
objects.

> > (It was enabled because we used to track shutdown leak metrics and
> > some other metrics, but when we switched to tracking
> > performance/memory metrics from graphs to tracking them via
> > regression notices, regression notices weren't added for those
> > measurements, so they just kept regressing without people noticing.
> > At some point we then stopped running them because nobody was
> > looking at them.  I wouldn't mind having that metric tracked again
> > in some way because it does catch some real leak regressions in
> > addition to shutdown leaks.)
> 
> Andrew McCreight is working on getting LSAN enabled by default on TBPL
> (bug 976414). He's already found a bunch of leaks using it; see the
> blocking bugs. LSAN's docs are here:
> https://code.google.com/p/address-sanitizer/wiki/LeakSanitizer.
> There's not much info on how it works, though I suspect it's extremely
> similar to how Valgrind's leak checking works, which is described in
> some detail here:
> http://www.valgrind.org/docs/manual/mc-manual.html#mc-manual.leaks
> 
> If you're still concerned, porting this feature of Trace Malloc to DMD
> is an option. But in general, tools like Valgrind and ASAN/LSAN do a
> better job of leak detection than simple malloc replacement tools like
> Trace Malloc, because they are able to do a conservative scan of
> addressable memory to find pointers. This means they can distinguish
> truly unreachable blocks from blocks that are still reachable but we
> didn't bother freeing. In practice, the number of blocks in the latter
> category is huge, and IME tools that can't distinguish the two cases
> are very difficult to use.

It isn't inherently huge; back when we were tracking the number it
was around 8K on Windows, much of which was NSPR, NSS, and PR Log
modules.  (It was larger than that on Linux and larger still on
Mac.)

The fact that we've regressed is part of what makes it less useful.

> >> There's also Leaky, which is documented on the abovementioned wiki
> >> page. I think it works in tandem with Trace Malloc, and may be a
> >> candidate for removal as well.
> >
> > I don't think leaky is in the tree anymore.  (It was once in
> > tools/leaky/.)
> 
> Oh, I thought that tools/jprof/leaky.{cpp,h} was it, but it looks like
> bug 750290 removed it. I guess I can remove the references from the
> MDN page. (I can probably remove Purify from that page, too!)

No, that's a copy of some of the leaky code that's used in jprof,
which is a working profiling tool.

-David

-- 
𝄞   L. David Baron http://dbaron.org/   𝄂
𝄢   Mozilla  https://www.mozilla.org/   𝄂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Do we still need Trace Malloc?

2014-05-19 Thread Nicholas Nethercote
On Mon, May 19, 2014 at 3:05 PM, L. David Baron  wrote:
>> Do we still need Trace Malloc?
>
> Are you talking about removing it from the debug builds done on our
> infra, or removing it from the tree?

I'm aiming for the latter, though the former is a reasonable first step :)

> I think the former is fine;
> I'd like to know more about the memory graph analysis abilities of
> ASAN before being ok with the latter.
>
> (It was enabled because we used to track shutdown leak metrics and
> some other metrics, but when we switched to tracking
> performance/memory metrics from graphs to tracking them via
> regression notices, regression notices weren't added for those
> measurements, so they just kept regressing without people noticing.
> At some point we then stopped running them because nobody was
> looking at them.  I wouldn't mind having that metric tracked again
> in some way because it does catch some real leak regressions in
> addition to shutdown leaks.)

Andrew McCreight is working on getting LSAN enabled by default on TBPL
(bug 976414). He's already found a bunch of leaks using it; see the
blocking bugs. LSAN's docs are here:
https://code.google.com/p/address-sanitizer/wiki/LeakSanitizer.
There's not much info on how it works, though I suspect it's extremely
similar to how Valgrind's leak checking works, which is described in
some detail here:
http://www.valgrind.org/docs/manual/mc-manual.html#mc-manual.leaks

If you're still concerned, porting this feature of Trace Malloc to DMD
is an option. But in general, tools like Valgrind and ASAN/LSAN do a
better job of leak detection than simple malloc replacement tools like
Trace Malloc, because they are able to do a conservative scan of
addressable memory to find pointers. This means they can distinguish
truly unreachable blocks from blocks that are still reachable but we
didn't bother freeing. In practice, the number of blocks in the latter
category is huge, and IME tools that can't distinguish the two cases
are very difficult to use.

>> There's also Leaky, which is documented on the abovementioned wiki
>> page. I think it works in tandem with Trace Malloc, and may be a
>> candidate for removal as well.
>
> I don't think leaky is in the tree anymore.  (It was once in
> tools/leaky/.)

Oh, I thought that tools/jprof/leaky.{cpp,h} was it, but it looks like
bug 750290 removed it. I guess I can remove the references from the
MDN page. (I can probably remove Purify from that page, too!)

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Do we still need Trace Malloc?

2014-05-19 Thread L. David Baron
On Monday 2014-05-19 07:25 -0700, Nicholas Nethercote wrote:
> Do we still need Trace Malloc? I suspect it's barely used these days.
> For memory profiling, we have about:memory and DMD. For shutdown leak
> detection we have ASAN and Valgrind.
> 
> Trace Malloc is documented here:
> https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/Debugging_memory_leaks#Trace_Malloc
> 
> Trace Malloc is enabled for all TBPL debug builds. I'm not sure why; I
> did a try run today where I disabled it and it was green.
> 
> It's used to get stacks within the deadlock detector, but I'm not sure
> if that's necessary, and it doesn't seem like it would be that hard to
> replace if it is necessary.

Are you talking about removing it from the debug builds done on our
infra, or removing it from the tree?  I think the former is fine;
I'd like to know more about the memory graph analysis abilities of
ASAN before being ok with the latter.

(It was enabled because we used to track shutdown leak metrics and
some other metrics, but when we switched to tracking
performance/memory metrics from graphs to tracking them via
regression notices, regression notices weren't added for those
measurements, so they just kept regressing without people noticing.
At some point we then stopped running them because nobody was
looking at them.  I wouldn't mind having that metric tracked again
in some way because it does catch some real leak regressions in
addition to shutdown leaks.)

> There's also Leaky, which is documented on the abovementioned wiki
> page. I think it works in tandem with Trace Malloc, and may be a
> candidate for removal as well.

I don't think leaky is in the tree anymore.  (It was once in
tools/leaky/.)

-David

-- 
𝄞   L. David Baron http://dbaron.org/   𝄂
𝄢   Mozilla  https://www.mozilla.org/   𝄂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Do we still need Trace Malloc?

2014-05-19 Thread Kyle Huey
On Mon, May 19, 2014 at 7:25 AM, Nicholas Nethercote
 wrote:
> Hi,
>
> Do we still need Trace Malloc? I suspect it's barely used these days.
> For memory profiling, we have about:memory and DMD. For shutdown leak
> detection we have ASAN and Valgrind.
>
> Trace Malloc is documented here:
> https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/Debugging_memory_leaks#Trace_Malloc
>
> Trace Malloc is enabled for all TBPL debug builds. I'm not sure why; I
> did a try run today where I disabled it and it was green.
>
> It's used to get stacks within the deadlock detector, but I'm not sure
> if that's necessary, and it doesn't seem like it would be that hard to
> replace if it is necessary.
>
> There's also Leaky, which is documented on the abovementioned wiki
> page. I think it works in tandem with Trace Malloc, and may be a
> candidate for removal as well.
>
> Thanks.
>
> Nick
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

Probably not.  As you say, if you really do want all the detail ASAN
covers your use case.  Otherwise, about:memory provides a far better
summary and diff view.  I think ultimately dbaron would need to sign
off on removing it but I have no objections.

- Kyle
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Do we still need Trace Malloc?

2014-05-19 Thread Nicholas Nethercote
Hi,

Do we still need Trace Malloc? I suspect it's barely used these days.
For memory profiling, we have about:memory and DMD. For shutdown leak
detection we have ASAN and Valgrind.

Trace Malloc is documented here:
https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/Debugging_memory_leaks#Trace_Malloc

Trace Malloc is enabled for all TBPL debug builds. I'm not sure why; I
did a try run today where I disabled it and it was green.

It's used to get stacks within the deadlock detector, but I'm not sure
if that's necessary, and it doesn't seem like it would be that hard to
replace if it is necessary.

There's also Leaky, which is documented on the abovementioned wiki
page. I think it works in tandem with Trace Malloc, and may be a
candidate for removal as well.

Thanks.

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform