Closed Bug 957021 Opened 6 years ago Closed 6 years ago

Memory reporter paths are all screwed up

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla29
Tracking Status
firefox27 --- verified
firefox28 --- verified
firefox29 --- verified

People

(Reporter: njn, Assigned: njn)

References

Details

(Whiteboard: [MemShrink:P1])

Attachments

(1 file)

I just noticed that some memory reporter paths are messed up. For example if I
install and run Chatzilla in a new profile, I get the following (reasonable)
output in about:memory's "ExplicitAllocations":

> ├───10.53 MB (10.26%) -- add-ons/{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}
> │   ├───8.90 MB (08.68%) -- window-objects
> │   │   ├──7.48 MB (07.30%) -- top(chrome://chatzilla/content/chatzilla.xul, id=20)/active/window(chrome://chatzilla/content/chatzilla.xul)
> │   │   │  ├──6.65 MB (06.49%) ++ js-compartment([System Principal], about:blank)
> │   │   │  └──0.83 MB (00.81%) ++ (4 tiny)
> │   │   └──1.42 MB (01.39%) -- top(chrome://chatzilla/content/output-window.html, id=22)/active
> │   │      ├──1.14 MB (01.11%) -- window(chrome://chatzilla/content/output-window.html)
> │   │      │  ├──0.68 MB (00.66%) ++ js-compartment([System Principal], chrome://chatzilla/content/output-window.html)
> │   │      │  ├──0.34 MB (00.33%) ++ layout
> │   │      │  ├──0.09 MB (00.09%) ++ dom
> │   │      │  ├──0.04 MB (00.03%) ── style-sheets
> │   │      │  └──0.00 MB (00.00%) ── property-tables
> │   │      └──0.28 MB (00.27%) ++ window(about:blank)

But I also get two bits of weird output in the "Other Measurements" section.
This:
 
> 1.62 MB (100.0%) -- add-ons
> └──1.62 MB (100.0%) -- {59c81df5-4b7a-477b-912d-4e0fdf64e5f2}/window-objects
>    ├──1.47 MB (90.60%) ++ top(chrome://chatzilla/content/chatzilla.xul, id=20)/js-zone(0x7fb42bc3d000)
>    └──0.15 MB (09.40%) ++ top(chrome://chatzilla/content/output-window.html, id=22)/js-zone(0x7fb427115800)

Why do we have a standalone add-ons sub-tree?

and this:
 
> 1,535 (100.0%) -- event-counts
> ├──1,141 (74.33%) ++ window-objects
> └────394 (25.67%) -- add-ons/{59c81df5-4b7a-477b-912d-4e0fdf64e5f2}/window-objects
>      ├──387 (25.21%) -- top(chrome://chatzilla/content/chatzilla.xul, id=20)/active/window(chrome://chatzilla/content/chatzilla.xul)/dom
>      │  ├──386 (25.15%) ── event-listeners
>      │  └────1 (00.07%) ── event-targets
>      └────7 (00.46%) ── top(chrome://chatzilla/content/output-window.html, id=22)/active/window(chrome://chatzilla/content/output-window.html)/dom/event-listeners

Why do we have add-ons stuff under event-counts?

Here's a non-addon-related one:

> 185.02 MB (100.0%) -- window-objects
> ├───78.77 MB (42.57%) ++ layout
> ├───61.72 MB (33.36%) ++ dom
> ├───12.79 MB (06.91%) ++
> top(https://mail.google.com/mail/u/0/?ui=2&shva=1#label/A-moz%2Fbugz/1436994dad794925, id=23)/js-zone(0x7f3044b04000)

Why is js-zone stuff under the window-objects summary tree?

The problem was introduced in bug 904720. Prior to that bug, we built up
|windowPath| gradually, starting from "explicit/". Crucially, at two different
points in the building up, we copy the partial path into a hash table for later
use by the JS engine reporters.

Bug 904720 changed things so that the "explicit/" was only added to a separate
variable, |explicitWindowPath|, which was constructed after the hash table
insertions. This means that paths put into the hash tables are missing the
"explicit/".

The effect in practice seems to be that all the "js-zone" measurements that
should be in "explicit allocations" end up in "other measurements", though
there may be other effects I haven't spotted yet.

This is bad. Fortunately the fix is simple.
> Why do we have add-ons stuff under event-counts?

Actually, that one's ok, but the other two examples are still bogus.
This patch reinstates the "explicit/" prefix from the start of the path
building, and then copies and modifies that path as necessary for the
"event-counts" path.
Attachment #8356431 - Flags: review?(khuey)
This will need backporting to Aurora28 and Beta27 and some B2G branches.
Whiteboard: [MemShrink] → [MemShrink:P1]
Before landing, I added three assertions that would have caught this problem:

https://hg.mozilla.org/integration/mozilla-inbound/rev/80f53ccf4ce2
https://hg.mozilla.org/mozilla-central/rev/80f53ccf4ce2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment on attachment 8356431 [details] [diff] [review]
Fix messed-up memory reporter paths.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 904720
User impact if declined: Memory reporting is screwed up in certain situations
Testing completed (on m-c, etc.): Landed on m-c
Risk to taking this patch (and alternatives if risky): Low risk.  This change only affects memory reporting code, so any fallout will be limited to about:memory users.
String or IDL/UUID changes made by this patch: None
Attachment #8356431 - Flags: approval-mozilla-beta?
Attachment #8356431 - Flags: approval-mozilla-aurora?
> User impact if declined: Memory reporting is screwed up in certain situations

Just to clarify: every time we measure, some of the memory reports get screwed up, which affects the top-line "explicit" number. I.e. this is a really bad regression that significantly harms about:memory's correctness.
Target Milestone: mozilla29 → ---
Attachment #8356431 - Flags: approval-mozilla-beta?
Attachment #8356431 - Flags: approval-mozilla-beta+
Attachment #8356431 - Flags: approval-mozilla-aurora?
Attachment #8356431 - Flags: approval-mozilla-aurora+
Target Milestone: --- → mozilla29
Do you have steps to reproduce this behaviour?
Flags: needinfo?(n.nethercote)
Keywords: steps-wanted
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #9)
> Do you have steps to reproduce this behaviour?

- Install Chatzilla.
- Restart Firefox, open a Chatzilla window (but no need to connect to anything).
- Visit about:memory, click "Measure".
- In the "Other Measurements" section (towards the bottom) you'll see a tree of measurements that says "add-ons" at its root. For example:

> 1.62 MB (100.0%) -- add-ons
> └──1.62 MB (100.0%) -- {59c81df5-4b7a-477b-912d-4e0fdf64e5f2}/window-objects
>    ├──1.47 MB (90.60%) ++ top(chrome://chatzilla/content/chatzilla.xul, id=20)/js-zone(0x7fb42bc3d000)
>    └──0.15 MB (09.40%) ++ top(chrome://chatzilla/content/output-window.html, id=22)/js-zone(0x7fb427115800)

- With the bug fixed, that tree won't appear in the "Other Measurements" section.
Flags: needinfo?(n.nethercote)
(In reply to Nicholas Nethercote from comment #4)
> Before landing, I added three assertions that would have caught this problem:
Note that we have a StringBeginsWith function; you don't need to write your own.
Flagging for verification with the steps in comment 10.
Keywords: steps-wantedverifyme
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (X11; Linux i686; rv:28.0) Gecko/20100101 Firefox/28.0

Reproduce with Nightly from 2014-01-06.
Verified as fixed with Firefox 27 beta 8 (Build ID: 20140120132616) and latest Aurora (Build ID: 20140121004017) with STR from comment 10: add-ons is present only under event-counts.
Verified as fixed with latest Aurora, on: Win 8 x64, Ubuntu 13.10 x86 and OS X 10.8.5, using the STR from comment 10: add-ons is present only under event-counts.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.