DAMP Perf regression on all devtools open tests (+2-5%, 10ms) due to additional panel triggering overflow menu
Categories
(DevTools :: General, defect, P3)
Tracking
(Not tracked)
People
(Reporter: jdescottes, Unassigned)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
135.80 KB,
image/png
|
Details |
Regression from Bug 1594885.
The What's new panel impacted all existing open tests. The slowdown is about 10ms for each test.
- alert https://treeherder.mozilla.org/perf.html#/alerts?id=23959&hideDwnToInv=0
- pushlog https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6a2d40014464a2bb1489921bd152d229e273d14d&tochange=f01d761796534e46b655e27a634d65ffa5934be9
This is either the cost of the additional tab to render, or the cost of isSupportedTarget:
isTargetSupported: function(target) {
// The panel is currently not localized and should only be displayed to
// english users. See Bug 1596038 for cleanup.
const isEnglishUser = Services.locale.negotiateLanguages(
["en"],
[Services.locale.appLocaleAsBCP47]
).length;
// In addition to the basic visibility switch preference, we also have a
// higher level preference to disable the whole panel regardless of other
// settings. Should be removed in Bug 1596037.
const isFeatureEnabled = Services.prefs.getBoolPref(
"devtools.whatsnew.feature-enabled",
false
);
// This panel should only be enabled for regular web toolboxes.
return target.isLocalTab && isEnglishUser && isFeatureEnabled;
},
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 1•5 years ago
|
||
Try pushes in progress to understand where the performance regression comes from:
- baseline https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a21f49c08bec2616b3b867556f9c4a67deb2cc1
- simpler isTargetSupported https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fa07e0753fb73ea64c49448806fa6e9f48ae497
- remove panel https://treeherder.mozilla.org/#/jobs?repo=try&revision=adbb8f47a9cc6091b1b4cf350c5db1534833052d
Reporter | ||
Comment 2•5 years ago
|
||
Results are in, the regression is purely related to adding the new panel: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&newProject=try&newRevision=adbb8f47a9cc6091b1b4cf350c5db1534833052d&originalSignature=1759151&newSignature=1759151&framework=12&originalRevision=8fa07e0753fb73ea64c49448806fa6e9f48ae497
Comment 3•5 years ago
|
||
Product was discussing hiding some of the less used panels (like the memory panel) by default, would that fix this regression?
Reporter | ||
Comment 4•5 years ago
|
||
(In reply to Panos Astithas (he/him) [:past] (please ni?) from comment #3)
Product was discussing hiding some of the less used panels (like the memory panel) by default, would that fix this regression?
It's very likely that the regression occurs because the panel goes into the "overflow" menu.
This depends on the browser size (1280px for DAMP), and it's not a real user facing regression.
We could fix the regression by hiding another panel, but if this theory is confirmed we can also simply accept the regression as the new baseline.
Probably a false alarm.
Reporter | ||
Comment 5•5 years ago
|
||
screenshot of DAMP running, as you can see next to the accessibility panel, we see the overflow menu icon, which means the What's New panel is overflowing and triggers the creation of this menu
Comment 6•5 years ago
|
||
Agreed, thanks Julian. I think it will be fair to treat this as a real regression if telemetry data shows a similar regression in the 95% of users after we ship (this window size might be more common than we think). Alternatively, if we have data on the most common window sizes and product can model the likely impact, we could consider that too.
Reporter | ||
Comment 7•5 years ago
|
||
I tried removing another panel (accessibility) and there is no clear performance impact. I think we can consider that the performance hit is purely due to the overflow menu. I think we should simply accept the hit and resolve as WONTFIX.
Unblocking the What's New meta.
Reporter | ||
Updated•5 years ago
|
Comment 8•5 years ago
|
||
Updated•5 years ago
|
Reporter | ||
Comment 9•4 years ago
|
||
This performance regression is only coming from the appearance of the overflow menu, no need to address this.
Updated•2 years ago
|
Description
•