Closed Bug 1246855 Opened 4 years ago Closed 4 years ago
Fix up Compartment
Private memory reporting
Bug 1214961 added some memory reporting of CompartmentPrivate. But there are two shortcomings: - CompartmentPrivate::SizeOfIncludingThis() only measures part of mWrappedJS. - The measurement happens upon compartment creation. This confuses DMD, which assumes that measurement occurs during reporting, rather than in advance. Please r? me in the future for patches that involve non-trivial memory reporting changes!
Oh, also: - The "private-data" measurement in about:memory was put in the "dom" sub-tree instead of the "js-compartment" sub-tree.
Measuring in advance confuses DMD. It's better to measure during reporting. The next patch will reinstate the measurement, but during reporting.
Attachment #8717304 - Flags: review?(terrence)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
terrence: "private-data" has this description: > Extra data attached to the compartment by XPConnect, including > wrapped-js that is local to a single compartment. I find this awkward and unclear, for two reasons: - Is "wrapped-js" an uncountable noun? Or should it be "a wrapped-js" or "the wrapped-js" or "each wrapped-js" or something else? - "that is local to a single compartment" -- this private data belongs to a single compartment. Is phrase this redundant? Alternative ways of writing it would be welcome. Thank you.
Attachment #8717305 - Flags: review?(terrence)
I have confirmed that this causes a bunch of heap blocks to move from "Unreported" to "Once-reported" in DMD.
Attachment #8717304 - Flags: review?(terrence) → review+
Comment on attachment 8717305 [details] [diff] [review] (part 2) - Measure CompartmentPrivates during memory reporting Review of attachment 8717305 [details] [diff] [review]: ----------------------------------------------------------------- I think either of the new phrasings you proposed is better than what's there now.
Attachment #8717305 - Flags: review?(terrence) → review+
In JSCompartment::addSizeOfIncludingThis() I had to change this: > JSRuntime* rt = runtimeFromMainThread(); > auto callback = rt->sizeOfIncludingThisCompartmentCallback; to this: > auto callback = runtime_->sizeOfIncludingThisCompartmentCallback; to avoid some assertion failures in js::CurrentThreadCanAccessRuntime().
https://hg.mozilla.org/integration/mozilla-inbound/rev/24529b78e2eb5ba8f4deb0b61810f248dc1705aa Bug 1246855 (part 1) - Remove the in-advance measurement of CompartmentPrivates. r=terrence. https://hg.mozilla.org/integration/mozilla-inbound/rev/99bef2f7a333546e90de4a11094b1ab3f507dcfe Bug 1246855 (part 2) - Measure CompartmentPrivates during memory reporting. r=terrence.
You need to log in before you can comment on or make changes to this bug.