Closed Bug 672439 Opened 11 years ago Closed 11 years ago
Avoid duplicate names for system compartment reporters
It's possible to have many compartments with the same codebase, and these all get lumped together in about:memory. This is particularly bad if you have a Jetpack-based add-on, because you can have dozens of compartments merged under the name "[System Principal]". It's misleading to lump them all together, we should separate them. The very simplest thing we could do is add the compartment's address as a suffix to its name. That'll give the required separation without giving any indication of where each duplicate compartment came from. Maybe we can do better, eg. indicate which system-principal compartments are sandboxes.
Here's the output of about:memory upon start-up with Bugzilla Tweaks installed, with the soon-to-be-attached patch applied.
This patch adds the address of a compartment, but only does it for system compartments, because (a) they're the only ones we're seeing lots of compartments with the same name, and (b) the addresses are a bit ugly, so we shouldn't show them where they're not useful. An example: │ ├──14.04 MB (20.68%) -- compartment([System Principal], 0x7ffb8c6a5000) │ │ ├───9.20 MB (13.56%) -- gc-heap │ │ │ ├──4.59 MB (06.76%) -- objects │ │ │ ├──2.15 MB (03.17%) -- shapes │ │ │ ├──1.70 MB (02.51%) -- strings │ │ │ ├──0.56 MB (00.82%) -- arena-unused │ │ │ └──0.20 MB (00.29%) -- (3 omitted) │ │ ├───2.02 MB (02.97%) -- scripts │ │ ├───0.97 MB (01.43%) -- string-chars │ │ ├───0.81 MB (01.20%) -- mjit-code │ │ ├───0.58 MB (00.85%) -- (3 omitted) │ │ └───0.46 MB (00.68%) -- object-slots And see the attachment for more examples.
Attachment #546715 - Flags: review?(gal)
Comment on attachment 546715 [details] [diff] [review] patch There is a special place reserved in hell for people who do hard-coding like this. Just say'n.
Attachment #546715 - Flags: review?(gal) → review+
Which hard-coding? I'm not relying on the name of the system principal, I'm using JSCompartment::isSystemCompartment. Indeed, the patch removes one place where the name was hard-coded, using SYSTEM_PRINCIPAL_SPEC instead.
Yeah, the original sin was done by gregor. We will try to add some clean API any-time-now (tm).
Gregor's hack was fixed by bug 671482. Bug 669123 is still open for a more general partitioning of compartments, should we desire it.
Summary: Avoid duplicate names for compartment reporters → Avoid duplicate names for system compartment reporters
Whiteboard: [MemShrink] → [MemShrink:P1]
Whiteboard: [MemShrink:P1] → [MemShrink:P1][inbound]
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.