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

RESOLVED FIXED in Firefox 58

Status

()

defect
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mconley, Assigned: enndeakin)

Tracking

(Blocks 1 bug, {stale-bug})

unspecified
mozilla58
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox58 fixed)

Details

(Whiteboard: [ohnoreflow][qf:p2][reserve-photon-performance])

Attachments

(1 attachment)

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
Assignee

Comment 1

2 years ago
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
Reporter

Updated

2 years ago
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]
Assignee

Comment 6

2 years ago
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+

Comment 8

2 years ago
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/61e07270c733
only adjust arrow position in panel once, r=dao

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/61e07270c733
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.