1441430 - Provide more detail about atoms memory usage

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Nicholas Nethercote [inactive]]

The old output had a single value "atoms-table". The new output looks like
this:

> 681,264 B (00.37%) -- atoms
> +--332,432 B (00.18%) -- dynamic-atoms
> +--262,176 B (00.14%) -- table
> +---86,656 B (00.05%) -- static-atoms

MozReview-Commit-ID: 924vUmxHAlh

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Provide more detail about atoms memory usage

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8954271 [details] [diff] [review]
Provide more detail about atoms memory usage

Review of attachment 8954271 [details] [diff] [review]:
-----------------------------------------------------------------

I trust that you can update this appropriately over bholley's sharded atom table patches.

::: xpcom/ds/nsAtomTable.h
@@ +28,5 @@
> +};
> +} // namespace mozilla
> +
> +void NS_AddSizeOfAtoms(mozilla::MallocSizeOf aMallocSizeOf,
> +                       mozilla::AtomsSizes& aSizes);

With discussions previously about returning values as values, rather than via outparams, should this just return mozilla::AtomsSizes?  Makes for slightly nicer code, and the actual assembly should be just as efficient.

Comment 3

[Nicholas Nethercote [inactive]]

> With discussions previously about returning values as values, rather than
> via outparams, should this just return mozilla::AtomsSizes?  Makes for
> slightly nicer code, and the actual assembly should be just as efficient.

The standard approach for memory reporting functions is to use the return value when measuring a single value, and a by-ref struct for multiple values. I'd prefer to stick to that... returning structs still feels weird.

Comment 4

[Nicholas Nethercote [inactive]]

> The standard approach for memory reporting functions is to use the return
> value when measuring a single value, and a by-ref struct for multiple
> values. I'd prefer to stick to that... returning structs still feels weird.

Also, after rebasing over bholley's patch, the measurement gets split up across multiple functions in a way that returning a struct would be awkward.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Bug 1441430 - Provide more detail about atoms memory usage.

The old output had a single value "atoms-table". The new output looks like
this:

> 649,904 B (00.39%) -- atoms
> ├──350,256 B (00.21%) -- dynamic
> │  ├──235,056 B (00.14%) ── unshared-buffers
> │  └──115,200 B (00.07%) ── atom-objects
> ├──212,992 B (00.13%) ── table
> └───86,656 B (00.05%) ── static/atom-objects

Review commit: https://reviewboard.mozilla.org/r/223672/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223672/

Comment 6

[Nicholas Nethercote [inactive]]

The new version has more detail.

Comment 7

[Nathan Froyd [:froydnj]]

Comment on attachment 8954581 [details]
Bug 1441430 - Provide more detail about atoms memory usage.

https://reviewboard.mozilla.org/r/223672/#review229738

Comment 8

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e2934676a8b1132723db6be553646d568b1dffc
Bug 1441430 - Provide more detail about atoms memory usage. r=froydnj

Comment 9

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/2e2934676a8b