Re: [dev-servo] Memory reporting in Servo: the next step

2017-10-06 Thread Anthony Ramine

> Le 6 oct. 2017 à 03:27, Nicholas Nethercote  a écrit :
> 
> I see two options.
> 
> - Overwrite the heapsize crate on crates.io with the malloc_size_of code. So
>  the crate name wouldn't change, but the API would change significantly,
> and
>  it would still be on crates.io. Then switch Servo over to using heapsize
>  everywhere.
> 
> - Switch Servo over to using malloc_size_of everywhere. (This leaves open
> the
>  question of what should happen to the heapsize crate.)
> 
> I personally prefer the second option, mostly because I view all of this
> code
> as basically unstable -- much like the allocator APIs in Rust itself -- and
> publishing it on crates.io makes me uneasy. Also, keeping the code in the
> tree
> makes it easier to modify.

I'm very strongly against moving more crates in servo/servo. Just publish the 
thing.

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


Re: [dev-servo] Memory reporting in Servo: the next step

2017-10-06 Thread Anthony Ramine

> Le 6 oct. 2017 à 07:58, Nicholas Nethercote  a écrit :
> 
> Not being able to use derive doesn't worry me, because it's just a
> convenience. I just looked through all the external crates that implement
> HeapSizeOf and hardly any are using derive.

Most don't use derive because they started depending on heapsize when derive 
plugins weren't yet stable. If we were to make them depend on heapsize now, 
they would probably use derive.
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo


Re: [dev-servo] Memory reporting in Servo: the next step

2017-10-06 Thread Nick Fitzgerald
Hi Nick!

The combination of deriving traits and Rust's ownership model is great for
memory reporting, and this stuff is _so_ much nicer to define than the
equivalent about:memory measurements in Firefox.

But it doesn't play out as well in the presence of a GC: ownership is
muddied. The Arc/Rc issues you mention are the tip of the iceberg, as
ownership is relaxed and heap edges proliferate. In the general case of
arbitrary edges that GCs support, one needs to analyze the heap graph to
get equivalent information about why something is alive and how much heap
space it is responsible for. Things like computing dominator trees and the
shortest path to some object from GC roots. For everything in Servo relying
on SpiderMonkey's GC, I think it makes sense to use the `JS::ubi::Node`
infrastructure[0] from SpiderMonkey, which gives a graph API to the GC
heap, and has various analyses (dominator trees, shortest paths, stack at
allocation time, etc) out of the box. This is what we use for the Firefox
DevTools' memory tool.

Of course, `JS::ubi::Node` is fairly heavily templated and has many inline
functions, so exposing this infrastructure to Rust will take some care and
effort.

Backing up a bit: there is a large benefit in separating the heap graph
traversal from the analyses run on the heap graph.

* With a separation, we can traverse the heap graph at some particular
point in time, serialize it to a core dump file, and then perform as much
post processing and analyzing as we like, without needing to reproduce that
exact heap graph over and over. This is similar to how rr separates
reproducing a bug from debugging it.

* With a separation, the bucketing of various types and their collective
heap size is just *one* analysis of the heap graph. We could have many
others, like the ones I've mentioned above: dominator trees, shortest path
to GC roots (I guess to a stack variable or global in general Rust),
dynamic leak detection[1], etc. The possibilities are open ended.

So maybe instead of using `JS::ubi::Node`, what do you think of creating
`#[derive(HeapGraph)]` and implementing `MallocSizeOf`, etc on top of it?

If `#[derive(HeapGraph)]` implemented `petgraph`'s graph traits, we would
even get shortest paths and dominators[2] for free.

Interested in your thoughts on the subject!

Cheers,

Nick

[0] http://searchfox.org/mozilla-central/source/js/public/UbiNode.h#32-164
[1] http://www.cs.utexas.edu/ftp/techreports/tr06-07.pdf
[2] https://github.com/bluss/petgraph/blob/master/src/algo/dominators.rs

On Thu, Oct 5, 2017 at 6:27 PM, Nicholas Nethercote 
wrote:

> Hi,
>
> Currently we have two similar but distinct approaches to measuring memory
> usage
> in Servo.
>
> - Servo uses the heapsize and heapsize_derive crates, from crates.io.
>
> - Gecko uses the malloc_size_of and malloc_size_of_derive crates, which are
> in
>   the tree.
>
> Because of this, you see this pattern quite a bit in style code:
>
> > #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
> > #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
> > struct Foo {
> > ...
> > }
>
> Why the difference? heapsize is the original approach. It sorta works, but
> has
> some big flaws. malloc_size_of is a redesign that addresses these flaws.
>
> - heapsize assumes you only want a single number for the size of any data
>   structure, when sometimes you want to break it down into different
> buckets.
>
>   malloc_size_of provides both "shallow" and "deep" measurements, which
> give
>   greater flexibilty, which helps with the multi-bucket case.
>
> - heapsize assumes jemalloc is the allocator. This causes build problems in
>   some configurations, e.g. https://github.com/servo/heapsize/issues/80.
>   It also means it doesn't integrate with DMD, which is the tool we use to
>   identify heap-unclassified memory in Firefox.
>
>   malloc_size_of doesn't assume a particular allocator. You pass in
> functions
>   that measure heap allocations. This avoids the build problems and also
> allows
>   integration with DMD.
>
> - heapsize doesn't measure HashMap/HashSet properly -- it computes an
> estimate
>   of the size, instead of getting the true size from the allocator. This
>   estimate can be (and in practice often will be) an underestimate.
>
>   malloc_size_of does measure HashMap/HashSet properly. However, this
> requires
>   that the allocator provide a function that can measure the size of an
>   allocation from an interior pointer. (Unlike Vec, HashMap/HashSet don't
>   provide a function that gives the raw pointer to the storage.) I had to
> add
>   support for this to mozjemalloc, and vanilla jemalloc doesn't support it.
> (I
>   guess we could fall back to computing the size when the allocator doesn't
>   support this, e.g. for Servo, which uses vanilla jemalloc.)
>
> - heapsize doesn't measure Rc/Arc properly -- currently it just defaults to
>   measuring through the Rc/Arc, which can lead to double-counting.
> Especially
>   when you use derive, wher

Re: [dev-servo] Memory reporting in Servo: the next step

2017-10-06 Thread Nicholas Nethercote
Memory reporting as done for about:memory is quite different to the memory
profiling done for devtools.

Here's how memory reporting works in SpiderMonkey: we iterate over every
cell in the GC heap, and measure any malloc'd things hanging of those
cells. That's it. It's not at all a standard GC type tracing algorithm
because it treats reachable and unreachable cells identically. It's simple
and works well and I don't want to change it.

Nick

On Sat, Oct 7, 2017 at 4:35 AM, Nick Fitzgerald 
wrote:

> Hi Nick!
>
> The combination of deriving traits and Rust's ownership model is great for
> memory reporting, and this stuff is _so_ much nicer to define than the
> equivalent about:memory measurements in Firefox.
>
> But it doesn't play out as well in the presence of a GC: ownership is
> muddied. The Arc/Rc issues you mention are the tip of the iceberg, as
> ownership is relaxed and heap edges proliferate. In the general case of
> arbitrary edges that GCs support, one needs to analyze the heap graph to
> get equivalent information about why something is alive and how much heap
> space it is responsible for. Things like computing dominator trees and the
> shortest path to some object from GC roots. For everything in Servo relying
> on SpiderMonkey's GC, I think it makes sense to use the `JS::ubi::Node`
> infrastructure[0] from SpiderMonkey, which gives a graph API to the GC
> heap, and has various analyses (dominator trees, shortest paths, stack at
> allocation time, etc) out of the box. This is what we use for the Firefox
> DevTools' memory tool.
>
> Of course, `JS::ubi::Node` is fairly heavily templated and has many inline
> functions, so exposing this infrastructure to Rust will take some care and
> effort.
>
> Backing up a bit: there is a large benefit in separating the heap graph
> traversal from the analyses run on the heap graph.
>
> * With a separation, we can traverse the heap graph at some particular
> point in time, serialize it to a core dump file, and then perform as much
> post processing and analyzing as we like, without needing to reproduce that
> exact heap graph over and over. This is similar to how rr separates
> reproducing a bug from debugging it.
>
> * With a separation, the bucketing of various types and their collective
> heap size is just *one* analysis of the heap graph. We could have many
> others, like the ones I've mentioned above: dominator trees, shortest path
> to GC roots (I guess to a stack variable or global in general Rust),
> dynamic leak detection[1], etc. The possibilities are open ended.
>
> So maybe instead of using `JS::ubi::Node`, what do you think of creating
> `#[derive(HeapGraph)]` and implementing `MallocSizeOf`, etc on top of it?
>
> If `#[derive(HeapGraph)]` implemented `petgraph`'s graph traits, we would
> even get shortest paths and dominators[2] for free.
>
> Interested in your thoughts on the subject!
>
> Cheers,
>
> Nick
>
> [0] http://searchfox.org/mozilla-central/source/js/public/UbiNode.h#32-164
> [1] http://www.cs.utexas.edu/ftp/techreports/tr06-07.pdf
> [2] https://github.com/bluss/petgraph/blob/master/src/algo/dominators.rs
>
> On Thu, Oct 5, 2017 at 6:27 PM, Nicholas Nethercote <
> n.netherc...@gmail.com>
> wrote:
>
> > Hi,
> >
> > Currently we have two similar but distinct approaches to measuring memory
> > usage
> > in Servo.
> >
> > - Servo uses the heapsize and heapsize_derive crates, from crates.io.
> >
> > - Gecko uses the malloc_size_of and malloc_size_of_derive crates, which
> are
> > in
> >   the tree.
> >
> > Because of this, you see this pattern quite a bit in style code:
> >
> > > #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
> > > #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
> > > struct Foo {
> > > ...
> > > }
> >
> > Why the difference? heapsize is the original approach. It sorta works,
> but
> > has
> > some big flaws. malloc_size_of is a redesign that addresses these flaws.
> >
> > - heapsize assumes you only want a single number for the size of any data
> >   structure, when sometimes you want to break it down into different
> > buckets.
> >
> >   malloc_size_of provides both "shallow" and "deep" measurements, which
> > give
> >   greater flexibilty, which helps with the multi-bucket case.
> >
> > - heapsize assumes jemalloc is the allocator. This causes build problems
> in
> >   some configurations, e.g. https://github.com/servo/heapsize/issues/80.
> >   It also means it doesn't integrate with DMD, which is the tool we use
> to
> >   identify heap-unclassified memory in Firefox.
> >
> >   malloc_size_of doesn't assume a particular allocator. You pass in
> > functions
> >   that measure heap allocations. This avoids the build problems and also
> > allows
> >   integration with DMD.
> >
> > - heapsize doesn't measure HashMap/HashSet properly -- it computes an
> > estimate
> >   of the size, instead of getting the true size from the allocator. This
> >   estimate can be (and in practice often will be) an underestimate.