Closed
Bug 1151750
Opened 9 years ago
Closed 9 years ago
Make it easier to find out why the AddonWatcher displays an add-on as slowing down Firefox
Categories
(Toolkit :: Performance Monitoring, defect)
Toolkit
Performance Monitoring
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: Yoric, Assigned: Yoric)
References
Details
Attachments
(2 files, 4 obsolete files)
For the moment, the only way to find out why the AddonWatcher has flagged an add-on as slow is to look at Telemetry. The self-support UX will certainly eventually provide something intended for end-users, but we need something for add-on developers. We should add the information in about:performance.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → dteller
Attachment #8589061 -
Flags: review?(dtownsend)
Assignee | ||
Comment 2•9 years ago
|
||
For reference, this is what it looks like.
Attachment #8589063 -
Flags: feedback?(dtownsend)
Comment 3•9 years ago
|
||
Comment on attachment 8589061 [details] [diff] [review] Recapitulating alerts in about:performance Review of attachment 8589061 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/aboutperformance/content/aboutPerformance.js @@ +122,5 @@ > + "Alerts", > + "Jank level alerts", > + "(highest jank)", > + "Cross-Process alerts", > + "(highest CPOW)" Not necessary for landing but seems like you could construct this from the MEASURES constant above. @@ +153,5 @@ > + // Display the name of the add-on > + let elName = document.createElement("td"); > + elAddon.appendChild(elName); > + AddonManager.getAddonByID(addonId, a => { > + elName.textContent = a?a.name:addonId Nit: spaces around operators @@ +155,5 @@ > + elAddon.appendChild(elName); > + AddonManager.getAddonByID(addonId, a => { > + elName.textContent = a?a.name:addonId > + }); > + Nit: whitespace ::: toolkit/modules/AddonWatcher.jsm @@ +183,1 @@ > // Report mibehaviors to the user. Typo: misbehaviors @@ +185,3 @@ > let reason = null; > > + for (let k of FILTERS) { s/k/filter/ @@ +187,5 @@ > + for (let k of FILTERS) { > + let peak = stats.peaks[k] || 0; > + stats.peaks[k] = Math.max(diff[k], peak); > + > + if (limits[k] < 0 || diff[k] <= limits[k]) { limits[k] <= 0 please or setting the prefs to 0 won't disable checking. @@ +193,2 @@ > } > + Nit: whitespace
Attachment #8589061 -
Flags: review?(dtownsend) → review+
Updated•9 years ago
|
Attachment #8589063 -
Flags: feedback?(dtownsend) → feedback+
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #3) > @@ +187,5 @@ > > + for (let k of FILTERS) { > > + let peak = stats.peaks[k] || 0; > > + stats.peaks[k] = Math.max(diff[k], peak); > > + > > + if (limits[k] < 0 || diff[k] <= limits[k]) { > > limits[k] <= 0 please or setting the prefs to 0 won't disable checking. Well, so far, we used -1 for disabling. Do you want me to change this?
Assignee | ||
Comment 5•9 years ago
|
||
Applied feedback. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3d558695b31
Attachment #8589061 -
Attachment is obsolete: true
Attachment #8590252 -
Flags: review+
Comment 6•9 years ago
|
||
(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #4) > (In reply to Dave Townsend [:mossop] from comment #3) > > @@ +187,5 @@ > > > + for (let k of FILTERS) { > > > + let peak = stats.peaks[k] || 0; > > > + stats.peaks[k] = Math.max(diff[k], peak); > > > + > > > + if (limits[k] < 0 || diff[k] <= limits[k]) { > > > > limits[k] <= 0 please or setting the prefs to 0 won't disable checking. > > Well, so far, we used -1 for disabling. Do you want me to change this? The previous code ignored limits <= 0 and I don't see a reason to change that.
Assignee | ||
Comment 7•9 years ago
|
||
My bad.
Assignee | ||
Comment 8•9 years ago
|
||
Applied final feedback.
Attachment #8590252 -
Attachment is obsolete: true
Assignee | ||
Comment 9•9 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3d558695b31
Assignee | ||
Comment 10•9 years ago
|
||
So, on Try, the MacOS X 10.8 build timeouts silently during linking, for no reason that I can understand. Since I have strictly no issue locally on MacOS X and other platforms pass all tests, I'll suspect an unrelated issue (disk full?) and mark this checkin-needed.
Keywords: checkin-needed
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/1786e8cb842e
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Comment 12•9 years ago
|
||
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=2635525&repo=fx-team
Flags: needinfo?(dteller)
Whiteboard: [fixed-in-fx-team]
Assignee | ||
Comment 13•9 years ago
|
||
Oh, wrong patch :/
Comment 14•9 years ago
|
||
This wasn't at fault. Re-landed. https://hg.mozilla.org/integration/fx-team/rev/886f4e8d4bc0
Comment 15•9 years ago
|
||
Except this is a real failure from this patch. Backed out again. https://hg.mozilla.org/integration/fx-team/rev/b87c799bfb90 https://treeherder.mozilla.org/logviewer.html#?job_id=2641082&repo=fx-team
Assignee | ||
Comment 16•9 years ago
|
||
Yes, as I wrote, wrong patch. My apologies.
Flags: needinfo?(dteller)
Assignee | ||
Comment 17•9 years ago
|
||
/r/7069 - Bug 1151750 - about:performance now recapitulates alerts;r=mossop /r/7071 - Bug 1152759 - Regroup Performance Monitoring modules/components;r=yoric /r/7073 - API WIP Pull down these commits: hg pull -r 516cbbd3805a58ffcc21feaf47c9ed66dfd29595 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8592864 -
Flags: review?(dtownsend)
Comment 18•9 years ago
|
||
Comment on attachment 8592864 [details] MozReview Request: bz://1151750/Yoric Assuming this is all the same as the patch in bug 1154239
Attachment #8592864 -
Flags: review?(dtownsend)
Assignee | ||
Comment 19•9 years ago
|
||
Rebased. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=76d251f8f314
Attachment #8590487 -
Attachment is obsolete: true
Attachment #8592864 -
Attachment is obsolete: true
Attachment #8594699 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
https://hg.mozilla.org/mozilla-central/rev/f35b2802480c
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla40
Blocks: 1157471
You need to log in
before you can comment on or make changes to this bug.
Description
•