Closed Bug 672439 Opened 9 years ago Closed 9 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:P1] → [MemShrink:P1][inbound]
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.