1189799 - about:performance shows duplicate lines for each addon (1 entry per parent + 1 content process)

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[Vladan Djeric (:vladan)]

An example...

Send Tab to Device currently performs well.less
    Full name: jid1-mdjmA7if6lo8lA@jetpack.
    Impact on framerate: 0/10.
    CPU usage: 0%.
    System usage: 0%.
    Blocking process calls: 0%.
    Measure start: 957 seconds ago.
    Process: 8516 (parent).

Send Tab to Device currently performs well.less
    Full name: jid1-mdjmA7if6lo8lA@jetpack.
    Impact on framerate: 0/10.
    CPU usage: 0%.
    System usage: 0%.
    Blocking process calls: 0%.
    Measure start: 957 seconds ago.
    Process: 8392 (child).

Send Tab to Device currently performs well.less
    Full name: jid1-mdjmA7if6lo8lA@jetpack.
    Impact on framerate: 0/10.
    CPU usage: 0%.
    System usage: 0%.
    Blocking process calls: 0%.
    Measure start: 957 seconds ago.
    Process: 7064 (child).

Send Tab to Device currently performs well.less
    Full name: jid1-mdjmA7if6lo8lA@jetpack.
    Impact on framerate: 0/10.
    CPU usage: 0%.
    System usage: 0%.
    Blocking process calls: 0%.
    Measure start: 957 seconds ago.
    Process: 4144 (child).

Comment 1

[David Teller [:Yoric] - still alive but not very active]

PerformanceStats could identify that the same addonId appears in different children, and sum the results. This may lead to CPU/System usage higher than 100%.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Actually, about:performance is probably better suited to grouping the add-ons for display.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Attachment: MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Attachment: MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Comment 6

[:Felipe Gomes (needinfo for replies!)]

I'm reviewing this and wrapping my head around the new data structure, but before continuing or trying to figure out I have a question that I think is easier to just ask you: I think there's still value in also seeing the values individually (per-process), specially to see the different performance impact of add-ons in the chrome and the content processes. So I guess what I wanted to ask is if this new structure will make it harder to do that or will that be ok? Not asking to build this visualization here in this bug but I think the UI in the future could allow you to opt between aggregate or per-process values

Comment 7

[David Teller [:Yoric] - still alive but not very active]

Actually, I hope that it will make it easier to display the information. Of course, the only way to be sure is to actually write that code :)

Comment 8

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8652388 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

https://reviewboard.mozilla.org/r/17149/#review15859

::: toolkit/components/perfmonitoring/AddonWatcher.jsm
(Diff revision 1)
> -            // We are only interested in add-ons.

q: what was the other thing that are not add-ons that is now included here? Won't this affect the behavior of AddonWatcher unexpectedly?

Comment 9

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8652387 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

https://reviewboard.mozilla.org/r/17147/#review15861

Comment 10

[David Teller [:Yoric] - still alive but not very active]

https://reviewboard.mozilla.org/r/17149/#review15859

> q: what was the other thing that are not add-ons that is now included here? Won't this affect the behavior of AddonWatcher unexpectedly?

Well, `snapshot.componentsData` contains add-ons and webpages (and, depending on options, built-in components). Prior to the patch, we iterated through all this list and skipped stuff that wasn't an add-on. With the patch, we iterate through `snapshot.addons`, which contains only the add-ons.

Comment 11

[David Teller [:Yoric] - still alive but not very active]

Attachment: MozReview Request: Bug 1200138 - AddonWatcher now uses groupId to subtract between two instances of an add-on;r=felipe

Bug 1200138 - AddonWatcher now uses groupId to subtract between two instances of an add-on;r=felipe

Comment 12

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8652387 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Comment 13

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8652388 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Comment 14

[David Teller [:Yoric] - still alive but not very active]

Attachment: MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r?felipe

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8655616 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

https://reviewboard.mozilla.org/r/17947/#review16139

Comment 16

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8652387 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Comment 17

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8652388 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Comment 18

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8655616 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

Comment 19

[David Teller [:Yoric] - still alive but not very active]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5afa32635045

Comment 20

[Carsten Book [:Tomcat]]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #18)
> Comment on attachment 8655616 [details]
> MozReview Request: Bug 1189799 - Make sure that about:performance displays
> each add-on only once (more tests);r=felipe
> 
> Bug 1189799 - Make sure that about:performance displays each add-on only
> once (more tests);r=felipe

Hey Yoric, this failed to apply:

patching file toolkit/components/perfmonitoring/AddonWatcher.jsm
Hunk #1 FAILED at 159
1 out of 1 hunks FAILED -- saving rejects to file toolkit/components/perfmonitoring/AddonWatcher.jsm.rej
patch failed to apply
abort: fix up the merge and run hg transplant --continue

could you guys take a look ? thanks!

Comment 21

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8652387 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Comment 22

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8652388 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Comment 23

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8655616 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

Comment 24

[David Teller [:Yoric] - still alive but not very active]

I seem to have a conflict somewhere. Investigating.

Comment 25

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8655616 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

Comment 26

[David Teller [:Yoric] - still alive but not very active]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4cc083c834e

Comment 27

[Carsten Book [:Tomcat]]

Yoric, so you only want to have attachment 8655616 [details] checked-right not the other 2 ones or ?

Comment 28

[David Teller [:Yoric] - still alive but not very active]

No, it's all three of them, why?

Comment 29

[Carsten Book [:Tomcat]]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #28)
> No, it's all three of them, why?

hm for the first checkin-request there seems to be problem and don't apply to fx-team cleanly:

patching file toolkit/components/aboutperformance/content/aboutPerformance.js
Hunk #2 FAILED at 73
Hunk #3 FAILED at 296
2 out of 8 hunks FAILED -- saving rejects to file toolkit/components/aboutperformance/content/aboutPerformance.js.rej
patch failed to apply


could you take a look, thanks!

Comment 30

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8652387 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (front-end);r=felipe

Comment 31

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8652388 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (back-end);r=felipe

Comment 32

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8655616 [details]
MozReview Request: Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

Bug 1189799 - Make sure that about:performance displays each add-on only once (more tests);r=felipe

Comment 33

[David Teller [:Yoric] - still alive but not very active]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=39d75b86e1d7

Comment 34

[David Teller [:Yoric] - still alive but not very active]

This should now be fixed.

Comment 35

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/0ba6203a0058
https://hg.mozilla.org/integration/fx-team/rev/5bdc9ee94664
https://hg.mozilla.org/integration/fx-team/rev/3525672b8b3b

Comment 36

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0ba6203a0058
https://hg.mozilla.org/mozilla-central/rev/5bdc9ee94664
https://hg.mozilla.org/mozilla-central/rev/3525672b8b3b

Comment 37

[Carsten Book [:Tomcat]]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #34)
> This should now be fixed.

yeah thanks Yoric!