Closed
Bug 1339859
Opened 7 years ago
Closed 7 years ago
[meta] Very slow width resize action when multiple tabs are pinned using compact themes
Categories
(Firefox :: Tabbed Browser, defect)
Tracking
()
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox51 | --- | unaffected |
firefox52 | --- | unaffected |
firefox53 | --- | wontfix |
firefox54 | --- | fix-optional |
firefox55 | --- | fixed |
People
(Reporter: bmaris, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: meta, Whiteboard: [photon-performance])
[Affected versions]: - latest Developer Edition 53.0a2 - latest Nightly 54.0a1 - Firefox 52 beta 6 using compact theme (https://addons.mozilla.org/en-Us/firefox/addon/devedition-theme-enabler/) [Affected platforms]: - macOS 10.12.3 [Steps to reproduce]: 1. Start Firefox 2. Change the theme to one of the compact themes (about:addons/appearance) 3. Open multiple tabs (eg.10) and pin them 4. Grab Fx from one side and shrink the width of the browser [Expected result]: - The transition is smooth [Actual result]: - Very laggy transition when changing the width of Fx. [Regression range]: - This is not a recent regression in Compact Themes feature since it reproduces in Nightly from 2017-01-17 as well. [Additional notes]: - Screencast showing the issue is attached. - I noted that 52 and 51 are not affected because to reproduce on those builds it is required to install the addon from amo. The tracking can be changed if considered otherwise.
Reporter | ||
Comment 1•7 years ago
|
||
And this is the screencast showing the issue (this was to large to upload it to Bugzilla). https://dl.dropboxusercontent.com/u/109148197/New%20slow%20transition.mov
Reporter | ||
Comment 2•7 years ago
|
||
This is not reproducible with other lightweight themes or Default theme.
Updated•7 years ago
|
Blocks: compact-themes, 1331679
Reporter | ||
Comment 4•7 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #3) > Could you reproduce this on Windows or Linux? No, this is only reproducible on macOS 10.12.3. I've also tested using Mac OS X 10.11.6 but was unable to see this issue.
Flags: needinfo?(bogdan.maris)
Comment 5•7 years ago
|
||
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #4) > (In reply to Dão Gottwald [:dao] from comment #3) > > Could you reproduce this on Windows or Linux? > > No, this is only reproducible on macOS 10.12.3. I've also tested using Mac > OS X 10.11.6 but was unable to see this issue. Have you tested multiple machines with 10.12.3? Seems unlikely (but possible!) this is an OS X version issue, and more likely that it's to do with the graphics card, 3rd party software, or something else specific to that machine.
Updated•7 years ago
|
Flags: needinfo?(bogdan.maris)
Reporter | ||
Comment 6•7 years ago
|
||
(In reply to :Gijs from comment #5) > (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #4) > > (In reply to Dão Gottwald [:dao] from comment #3) > > > Could you reproduce this on Windows or Linux? > > > > No, this is only reproducible on macOS 10.12.3. I've also tested using Mac > > OS X 10.11.6 but was unable to see this issue. > > Have you tested multiple machines with 10.12.3? Seems unlikely (but > possible!) this is an OS X version issue, and more likely that it's to do > with the graphics card, 3rd party software, or something else specific to > that machine. I tested on two different iMacs and one MacBook Pro (10.12.1, 10.12.3 and 10.12.4): - Intel Core i5 2.7 GHz, graphics Intel Iris Pro - Intel Core i5 2.5 GHz, AMD Radeon HD 6750M - Intel Core i5 2.7 Ghz, Intel Iris Graphics 6100 1536MB
Flags: needinfo?(bogdan.maris) → needinfo?(gijskruitbosch+bugs)
Reporter | ||
Comment 7•7 years ago
|
||
The issue is reproducible on all three machines from above.
Comment 8•7 years ago
|
||
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #6) > (In reply to :Gijs from comment #5) > > (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #4) > > > (In reply to Dão Gottwald [:dao] from comment #3) > > > > Could you reproduce this on Windows or Linux? > > > > > > No, this is only reproducible on macOS 10.12.3. I've also tested using Mac > > > OS X 10.11.6 but was unable to see this issue. > > > > Have you tested multiple machines with 10.12.3? Seems unlikely (but > > possible!) this is an OS X version issue, and more likely that it's to do > > with the graphics card, 3rd party software, or something else specific to > > that machine. > > I tested on two different iMacs and one MacBook Pro (10.12.1, 10.12.3 and > 10.12.4): > - Intel Core i5 2.7 GHz, graphics Intel Iris Pro > - Intel Core i5 2.5 GHz, AMD Radeon HD 6750M > - Intel Core i5 2.7 Ghz, Intel Iris Graphics 6100 1536MB This sounds like a native widget issue, then. Markus, can you take a look or redirect to someone who can?
Component: Theme → Widget: Cocoa
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(mstange)
Product: Firefox → Core
Comment 9•7 years ago
|
||
I'm not able to reproduce this bug on my machine, using macOS 10.12.3 Beta (16D25a). Bogdan, can you get a profile of the problem using the addon from https://perf-html.io/ and share the link to it?
Flags: needinfo?(mstange) → needinfo?(bogdan.maris)
Comment 10•7 years ago
|
||
Setting priority to P3 because this seems to affect only specific versions of OS X. This does not block.
Priority: -- → P3
Reporter | ||
Comment 11•7 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #9) > I'm not able to reproduce this bug on my machine, using macOS 10.12.3 Beta > (16D25a). > > Bogdan, can you get a profile of the problem using the addon from > https://perf-html.io/ and share the link to it? Sorry for the late reply. Here is the link requested: https://perfht.ml/2moFhcP
Flags: needinfo?(bogdan.maris) → needinfo?(mstange)
Comment 12•7 years ago
|
||
Thanks for the profile! We have a layout thrashing loop at http://searchfox.org/mozilla-central/rev/fcd9f1480d65f1a5df2acda97eb07a7e133f6ed4/browser/base/content/tabbrowser.xml#6078-6082 : for (let i = numPinned - 1; i >= 0; i--) { let tab = this.childNodes[i]; width += tab.getBoundingClientRect().width; tab.style.marginInlineStart = -(width + scrollButtonWidth + paddingStart) + "px"; } We query layout information and then set a CSS property that affects layout, in a loop.
Component: Widget: Cocoa → Tabbed Browser
Flags: needinfo?(mstange) → needinfo?(dao+bmo)
Product: Core → Firefox
Comment 13•7 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #12) > Thanks for the profile! > > We have a layout thrashing loop at > http://searchfox.org/mozilla-central/rev/ > fcd9f1480d65f1a5df2acda97eb07a7e133f6ed4/browser/base/content/tabbrowser. > xml#6078-6082 : > > for (let i = numPinned - 1; i >= 0; i--) { > let tab = this.childNodes[i]; > width += tab.getBoundingClientRect().width; > tab.style.marginInlineStart = -(width + scrollButtonWidth + paddingStart) > + "px"; > } > > We query layout information and then set a CSS property that affects layout, > in a loop. Resizing a window should call _positionPinnedTabs once at most if and when it causes the tab strip to overflow or underflow (and this isn't specific to compact themes). So that seems unlikely to be the biggest problem here if the whole resize operation is janky. Or are you seeing that _positionPinnedTabs is called multiple times?
Flags: needinfo?(dao+bmo) → needinfo?(mstange)
Comment 14•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #13) > Resizing a window should call _positionPinnedTabs once at most if and when > it causes the tab strip to overflow or underflow (and this isn't specific to > compact themes). It shouldn't be specific to Mac either. Has somebody reproduced this elsewhere?
Flags: needinfo?(bogdan.maris)
Comment 15•7 years ago
|
||
What I'm seeing in the profile is that each refresh tick during the resize takes 50ms: https://perfht.ml/2oLUkPa And 25ms of each of those is spent within _positionPinnedTabs. It's probably just one call to _positionPinnedTabs per refresh tick, but *inside* of that function there is a loop. We flush and invalidate styles for each pinned tab, which adds up if there are lots of them. I'm also seeing lots of time spent in frame construction. Are we toggling the "position" property on these tabs between "static" and "fixed"? Or are we removing tabs from the DOM and re-inserting them? Both of those require frame reconstruction. And the other problem I see in the profile, which is probably not caused by front-end code, is that during the resize there are very long gaps between calls to -[NSWindow setFrame:display:]. That function is where the window size actually changes. The time between those calls is filled with refresh driver ticks. Maybe there's an animation running, and we desperately try to repaint that animation multiple times before giving the system a chance to actually resize the window? But that doesn't explain why the "overflow" event handler ("onxbloverflow") is called on each of those refresh ticks; the overflow event shouldn't need to fire when the window size hasn't changed. So it seems there are multiple bugs here: Slow overflow event handlers (front-end specific) + starved window resizing while there are remaining slow Gecko events in the Gecko event loop (Mac-specific).
Flags: needinfo?(mstange)
Comment 16•7 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #15) > What I'm seeing in the profile is that each refresh tick during the resize > takes 50ms: https://perfht.ml/2oLUkPa > And 25ms of each of those is spent within _positionPinnedTabs. It's probably > just one call to _positionPinnedTabs per refresh tick, but *inside* of that > function there is a loop. We flush and invalidate styles for each pinned > tab, which adds up if there are lots of them. So _positionPinnedTabs is called way too often. This is definitely broken. > I'm also seeing lots of time spent in frame construction. Are we toggling > the "position" property on these tabs between "static" and "fixed"? Yes, but it's (supposed to be) a one-time switch when overflowing / underflowing. > But that doesn't explain why the "overflow" > event handler ("onxbloverflow") is called on each of those refresh ticks; > the overflow event shouldn't need to fire when the window size hasn't > changed. There's an overflow event commonly originating from tab elements (from bug 658467), but the overflow handler bails out early in that case. It sounds like in this case we're getting other unexpected overflow events.
Blocks: photon-performance-triage
Comment 17•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #14) > (In reply to Dão Gottwald [::dao] from comment #13) > > Resizing a window should call _positionPinnedTabs once at most if and when > > it causes the tab strip to overflow or underflow (and this isn't specific to > > compact themes). > > It shouldn't be specific to Mac either. Has somebody reproduced this > elsewhere? Already answered in comment 4.
Flags: needinfo?(bogdan.maris)
OS: All → Mac OS X
Comment 18•7 years ago
|
||
Dão, was there more to do here besides the deps that already got fixed?
Flags: needinfo?(dao+bmo)
Updated•7 years ago
|
Flags: qe-verify?
Priority: P3 → P2
Whiteboard: [photon-performance]
Comment 19•7 years ago
|
||
(In reply to :Gijs from comment #18) > Dão, was there more to do here besides the deps that already got fixed? We'll need confirmation from Bogdan on whether the symptom is fixed, and another profile to see if _positionPinnedTabs is still called too often, and if so figure out why.
Flags: needinfo?(dao+bmo) → needinfo?(bogdan.maris)
Reporter | ||
Comment 20•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #19) > (In reply to :Gijs from comment #18) > > Dão, was there more to do here besides the deps that already got fixed? > > We'll need confirmation from Bogdan on whether the symptom is fixed, and > another profile to see if _positionPinnedTabs is still called too often, and > if so figure out why. I just tried again using latest Nightly 55.0a1 (buildID: 20170417030204) on macOS 10.12.4 and I can't reproduce this issue anymore (Firefox is no longer slow on resize). Here is another profile I used: https://perfht.ml/2pcEtJF
Flags: needinfo?(bogdan.maris) → needinfo?(dao+bmo)
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Flags: needinfo?(dao+bmo)
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment 21•7 years ago
|
||
Discussed with Florian - setting as a meta bug.
Flags: qe-verify? → qe-verify-
Keywords: meta
Priority: P2 → --
QA Contact: bogdan.maris
Summary: Very slow width resize action when multiple tabs are pinned using compact themes → [meta] Very slow width resize action when multiple tabs are pinned using compact themes
Updated•7 years ago
|
No longer blocks: photon-performance-triage
Updated•7 years ago
|
Blocks: photon-performance-triage
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•