Closed Bug 1356674 Opened 4 years ago Closed 3 years ago

32ms uninterruptible reflow at get_alignmentPosition@chrome://global/content/bindings/popup.xml:158:11

Categories

(Toolkit :: XUL Widgets, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: mconley, Assigned: enndeakin)

References

Details

(Keywords: stale-bug, Whiteboard: [ohnoreflow][qf:p2][reserve-photon-performance])

Attachments

(1 file)

Here's the stack:

get_alignmentPosition@chrome://global/content/bindings/popup.xml:158:11
adjustArrowPosition@chrome://global/content/bindings/popup.xml:428:13
_syncContainerWithMainView@chrome://browser/content/customizableui/panelUI.xml:409:13
waitForSyncCallback@resource://services-common/async.js:98:7
Res__request@resource://services-sync/resource.js:386:14
Res_get@resource://services-sync/resource.js:415:12
_fetchInfo@resource://services-sync/service.js:466:14
sync@resource://services-sync/stages/enginesync.js:82:16
onNotify@resource://services-sync/service.js:1073:7
WrappedNotify@resource://services-sync/util.js:163:21
WrappedLock@resource://services-sync/util.js:119:16
_lockedSync@resource://services-sync/service.js:1063:12
sync/<@resource://services-sync/service.js:1055:7
WrappedCatch@resource://services-sync/util.js:94:16
sync@resource://services-sync/service.js:1044:5
syncTabs/</<@resource://services-sync/SyncedTabs.jsm:178:11
Flags: qe-verify?
Priority: -- → P2
Is this pretty much the same as 1296442?
Flags: needinfo?(mconley)
Possibly, yes. I _suspect_ this will go away once bug 1009116 is fixed.
Depends on: 1009116
Flags: needinfo?(mconley)
Please correct if this isn't the right component. Thanks!
Component: Untriaged → XUL Widgets
Product: Firefox → Toolkit
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][qf:p1][photon-performance]
Somewhat different stack @ 1.37ms:

get_alignmentPosition@chrome://global/content/bindings/popup.xml:158:11
adjustArrowPosition@chrome://global/content/bindings/popup.xml:428:13
onxblpopupshowing@chrome://global/content/bindings/popup.xml:483:9
openPopup@chrome://global/content/bindings/popup.xml:53:15
PanelUI.showSubView<@chrome://browser/content/customizableui/panelUI.js:519:7
TaskImpl_run@resource://gre/modules/Task.jsm:319:42
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
TaskImpl_run@resource://gre/modules/Task.jsm:324:15
TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:401:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:15
promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:396:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:15
TaskImpl@resource://gre/modules/Task.jsm:277:3
asyncFunction@resource://gre/modules/Task.jsm:252:14
Task_spawn@resource://gre/modules/Task.jsm:166:12
attach@resource:///modules/ExtensionPopups.jsm:448:12
onViewShowing@chrome://browser/content/ext-browserAction.js:160:37
wrapWidgetEventHandler/aWidget[aEventName]@resource:///modules/CustomizableUI.jsm:2400:16
PanelUI.showSubView<@chrome://browser/content/customizableui/panelUI.js:491:7
TaskImpl_run@resource://gre/modules/Task.jsm:319:42
TaskImpl@resource://gre/modules/Task.jsm:277:3
asyncFunction@resource://gre/modules/Task.jsm:252:14
handleWidgetCommand@resource:///modules/CustomizableUI.jsm:1501:7
EventListener.handleEvent*buildWidget@resource:///modules/CustomizableUI.jsm:1374:7
getWidgetNode@resource:///modules/CustomizableUI.jsm:887:16
insertNodeInWindow@resource:///modules/CustomizableUI.jsm:1140:26
insertNode@resource:///modules/CustomizableUI.jsm:1126:7
onWidgetAdded@resource:///modules/CustomizableUI.jsm:932:5
notifyListeners@resource:///modules/CustomizableUI.jsm:2102:11
createWidget@resource:///modules/CustomizableUI.jsm:2212:9
createWidget@resource:///modules/CustomizableUI.jsm:3256:7
build@chrome://browser/content/ext-browserAction.js:105:18
onManifestEntry@chrome://browser/content/ext-browserAction.js:91:5
asyncEmitManifestEntry@resource://gre/modules/ExtensionCommon.jsm:878:14
async*runManifest@resource://gre/modules/Extension.jsm:905:23
startup@resource://gre/modules/Extension.jsm:991:13
async*startup/this.startupPromise<@resource://gre/modules/LegacyExtensionsUtils.jsm:180:7
startup@resource://gre/modules/LegacyExtensionsUtils.jsm:135:27
startup@resource://gre/modules/addons/XPIProvider.jsm:4985:28
startup@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///D:/Firefox/extensions/uBlock0-webext@raymondhill.net.xpi!/bootstrap.js:33:5
callBootstrapMethod@resource://gre/modules/addons/XPIProvider.jsm:4998:11
startup@resource://gre/modules/addons/XPIProvider.jsm:2899:13
callProvider@resource://gre/modules/AddonManager.jsm:268:12
_startProvider@resource://gre/modules/AddonManager.jsm:737:5
startup@resource://gre/modules/AddonManager.jsm:904:9
startup@resource://gre/modules/AddonManager.jsm:3080:5
Another stack:

get_alignmentPosition@chrome://global/content/bindings/popup.xml:158:11
adjustArrowPosition@chrome://global/content/bindings/popup.xml:428:13
onxblpopupshowing@chrome://global/content/bindings/popup.xml:483:9
openPopup@chrome://global/content/bindings/popup.xml:53:15
showPopup@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1254:5
handleEvent/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1228:9
_createTimeout/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1482:7
setTimeout handler*_createTimeout@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1481:33
handleEvent@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1227:7
EventListener.handleEvent*_configurePlusButtonMenuElement@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1264:7
_configurePlusButtonMenu@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1288:5
configure@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1210:7
_configureWindow@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1003:12
_configureWindows@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:997:21
init@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:244:5
exports.main@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:1590:3
run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/addon/runner.js:149:7
startup/</<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/addon/runner.js:83:9
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
promise callback*completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:698:7
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:958:5
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
listener/<@resource://gre/modules/sdk/system/Startup.js:52:44
Flags: qe-verify? → qe-verify-
Priority: P2 → P3
Whiteboard: [ohnoreflow][qf:p1][photon-performance] → [ohnoreflow][qf:p1][reserve-photon-performance]
Whiteboard: [ohnoreflow][qf:p1][reserve-photon-performance] → [ohnoreflow][qf:p2][reserve-photon-performance]
This patch removes the call to adjustingArrowPosition within popupshowing and only does it in popuppositioned.

Some tests are adjusted to account for the reflows that don't occur. Running toolkit tests with this change reduces the number of popup layouts that happen by 10%.

No failures that I can see:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0af6672f8d0d492dd2f8d00c80bad2554e77e4c
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #8906627 - Flags: review?(dao+bmo)
Priority: P3 → P1
Comment on attachment 8906627 [details] [diff] [review]
Reduce calls to adjustArrowPosition

Review of attachment 8906627 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/performance/browser_appmenu_reflows.js
@@ +38,5 @@
>        "handleEvent@resource:///modules/PanelMultiView.jsm",
>        "openPopup@chrome://global/content/bindings/popup.xml",
>      ],
>  
> +    times: 7, // This number should only ever go down - never up.

While this number shouldn't be going up, I see that we're dropping 2 other reflows, so we have a net win of 1 reflow, which is acceptable, imo.
Attachment #8906627 - Flags: review?(dao+bmo) → review+
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/61e07270c733
only adjust arrow position in panel once, r=dao
https://hg.mozilla.org/mozilla-central/rev/61e07270c733
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.