1066303 - We should have a cleaner way of creating summaries that don't follow the structure of the explicit tree

Status: NEW

(Toolkit :: about:memory, defect)

Description

[Seth Fowler [:seth] [:s2h]]

So right now when doing memory reporting in imagelib, we report using a tree structure that looks like this:

images/[chrome|content]/[raster|vector]/[used|unused]/image(<URI>)/[source|decoded-heap|decoded-nonheap]

This very nicely gives us totals for e.g. chrome vs content, raster vs vector, used vs. unused, and for each image URI.

However, there's one category of totals that we want that we don't get for free from the above. It corresponds to this tree structure:

images/[chrome|content]/[raster|vector]/[used|unused]/[source|decoded-heap|decoded-nonheap]

The key difference is that the "image()" nodes have been removed, so we're summarizing over all images in each category.

Because this doesn't follow the structure of the tree we're actually using, we have to handle this by computing our own totals and reporting them manually. This complicates the code enough that it feels like a barrier to adding more summaries like these; I can only imagine what things would look like if we had several more!

I think that we should add a new feature to the memory reporting code to make this cleaner.

I'm not very familiar with that code, so I don't have a detailed, concrete suggestion here, but I discussed this idea on IRC with erahm and mccr8 and I think mccr8's suggestion of being able to tag a report as belonging to another category as well is a good one.

What this might look like in practice is that we'd provide one or more additional category paths when calling the memory reporter callback. It could be that we'd allow multiple reports for the same category, summing them all together, but I think we could also make it work using one report per category, as we (I think) do now, by just adding appropriate suffixes to the category path. So in imagelib we might report our values using the primary category I gave above (the first one) and a secondary category like this:

images/[chrome|content]/[raster|vector]/[used|unused]/[source|decoded-heap|decoded-nonheap]/image(<URI>)

The usual about:memory summarization will then do the rest of the work.

Comment 1

[Nicholas Nethercote [inactive]]

This suggestion immediately makes me think of the difficulties:

- We have *lots* of reporters, so adding extra flags is a pain.

- I like keeping about:memory as dumb as possible. Currently "explicit"-prefixed paths get one kind of special treatment, and the "heap-allocated" gets special treatment, and everything else can be treated the same. I'm reluctant to change that.

- Doing this manually doesn't seem that bad to me.

But perhaps I'm just so used to the current code that I don't see its flaws the way a fresh pair of eyes does. The devil will be in the details, though.

Any such change will need to handle all the aggregate trees in the "Other Measurements" section: images, window-objects, js-main-runtime. And maybe js-main-runtime-gc-heap-committed, though it's a bit different.

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

For what it's worth, there's a similar kind of thing going on with the B2G system memory reporter — there's a second tree with totals summed over all processes (and more/less specific in a few minor ways specific to that problem domain).  I've wondered if there could be some value in considering certain kinds of memory usage as a multidimensional array rather than (or as a generalization of) a tree.

One difference that comes to mind is that the system memory reporter deals with proportional set size, where pages shared among processes have 1/N of their size ascribed to each of their N mappings, so one thing those sums do is to add those pieces back together.