799019 - Tweak per-compartment memory reporting

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Patch coming shortly.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Tweak per-compartment memory reporting.

This patch:

- Creates Compartment::sizeOfIncludingThis() and moves some pre-existing code
  into it.

- Adds new measurement of Compartment::regExps.  Note that I'm measuring the
  hash table itself, but not measuring any of the RegExpShared objects
  pointed to by the table;  DMD indicates there's no need.

- Adds new measurement of Compartment::debuggees.

- Moves the memory reporting of JSCompartment objects themselves from
  runtime/compartment-objects to $COMPARTMENT/compartment-objects, which is a
  more logical place for it.

The two new reporters improve coverage of 64-bit desktop start-up by about
200KB, i.e. roughly 0.5%.

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 669028 [details] [diff] [review]
Tweak per-compartment memory reporting.

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

Gah, how did I end up owning this?

::: js/src/jscompartment.cpp
@@ +855,4 @@
>  {
> +    *compartmentObject = mallocSizeOf(this);
> +    sizeOfTypeInferenceData(tiSizes, mallocSizeOf);
> +    *shapesCompartmentTables = baseShapes.sizeOfExcludingThis(mallocSizeOf)

I have to admit that I have no idea what "shapesCompartmentTables" refers to.

Comment 3

[Nicholas Nethercote [inactive]]

> Gah, how did I end up owning this?

IIRC, you were the one who created jsmemorymetrics.cpp :)  Thanks for reviewing, and quickly.  I can ask others to review these kinds of changes in the future if you don't want to.

> I have to admit that I have no idea what "shapesCompartmentTables" refers to.

It shows up in about:memory under |compartment(foo)/shapes-extra/compartment-tables|.

BTW, I used DMD to find and check these changes, which should give you some added confidence!

Comment 4

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2494d51c3dc5

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2494d51c3dc5

Comment 6

[Justin Lebar (not reading bugmail)]

Comment on attachment 669028 [details] [diff] [review]
Tweak per-compartment memory reporting.

[Approval Request Comment]
This is needed to make the changes in bug 801780 apply cleanly.  I can work around this, but I think it's less risky to take this patch.

It's also an improvement with respect to bug 798484, which is important for B2G.

Relatively low-risk patch, and taking it keeps aurora and nightly in sync, reducing pain for B2G.

Comment 7

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4ff2c203af97