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

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect, P1)

Description

[Mike Conley (:mconley) (:⚙️)]

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

Comment 1

[Neil Deakin]

Is this pretty much the same as 1296442?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Possibly, yes. I _suspect_ this will go away once bug 1009116 is fixed.

Comment 3

[Stefan [:StefanG_QA]]

Please correct if this isn't the right component. Thanks!

Comment 4

[Brian Carpenter [:geeknik]]

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/[e-mail]!/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

Comment 5

[Brian Carpenter [:geeknik]]

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

Comment 6

[Neil Deakin]

Attachment: Reduce calls to adjustArrowPosition

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

Comment 7

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/61e07270c733
only adjust arrow position in panel once, r=dao

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/61e07270c733