Last Comment Bug 799019 - Tweak per-compartment memory reporting
: Tweak per-compartment memory reporting
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla19
Assigned To: Nicholas Nethercote [:njn]
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: DarkMatter
  Show dependency treegraph
Reported: 2012-10-07 23:05 PDT by Nicholas Nethercote [:njn]
Modified: 2012-10-19 21:15 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Tweak per-compartment memory reporting. (13.40 KB, patch)
2012-10-07 23:05 PDT, Nicholas Nethercote [:njn]
Ms2ger: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Nicholas Nethercote [:njn] 2012-10-07 23:05:15 PDT
Patch coming shortly.
Comment 1 Nicholas Nethercote [:njn] 2012-10-07 23:05:55 PDT
Created attachment 669028 [details] [diff] [review]
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 (⌚ UTC+1/+2) 2012-10-08 02:22:16 PDT
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 [:njn] 2012-10-08 15:05:51 PDT
> 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 [:njn] 2012-10-08 15:25:34 PDT
Comment 5 Ed Morley [:emorley] 2012-10-09 07:33:08 PDT
Comment 6 Justin Lebar (not reading bugmail) 2012-10-18 07:36:52 PDT
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) 2012-10-19 21:15:19 PDT

Note You need to log in before you can comment on or make changes to this bug.