Closed Bug 832641 Opened 7 years ago Closed 7 years ago
Menu items slow to paint/respond after peeking their sub-menu popups
This happens in Firefox 18 and current nightly. Tested and reproduced on Windows (HW accel either on or off) and Linux (Xubuntu 12.10 fully updated). Steps to reproduce: 1. Restart the browser, make sure the menu is visible, have many (30 at least) bookmarks folders at the bookmarks menu. 2. Hover all the main menu items with mouse, note that it responds fast with highlight (on Linux - click one of the main menu items first such that it responds to hovering different top menu items). 3. Click the bookmarks menu, hover each folder such that the subfolder popup shows. 4. Collapse the bookmarks menu, hover all the main menu items one after the other again. Expected result: - Main menu items respond quickly with highlight. Actual result: - The bookmarks menu is very slow to highlight or respond (could easily get over 1 second). - This also happens with bookmark toolbar folders. Other symptoms of this issue: - Resizing becomes very slow on windows, as apparently the menu is repainted on each resize paint. Workaround: - Hide the menu (or bookmarks toolbar) and show it again -> all menu items respond fast again, window resizing is fast again. Observations: - On windows, I noticed that each subfolder peek adds few GDI objects which are never released. - If a subfolder was already peeked before, it doesn't add GDI handles, even if the menu was collapsed after peeking and then peeked again. - I suspect (though haven't tested) that livemarks accumulate this issue each time they're updated and peeked. - Looking at the profiler after hovering slow main menu items, it appears that the callstack is in Paint most of the time, probably of the collapsed subfolder popups. Personally, I restarted Firefox many times due to degrading responsiveness over time. I think this could be one of the reasons (if not the main ones) for this issue. This might be related to the fact that I use livemarks regularly.
Summary: Menu items slow to respond after peeking their sub-menu popups → Menu items slow to paint/respond after peeking their sub-menu popups
I see that nsMenuFrame (and nsMenuTimerMediator) are constructed per menu item (and sub menu item etc) on the first time the item is displayed, but are not destructed when the menu collapses (the exception would be "Live" menus, like History and recently visited, whose items are destructed the next time the menu is expanded, just before it re-populates with updated items). This is probably by design to save construction at the next time menu items are displayed. So the bug could be that all the sub-items (recursively) are possibly repainted/composed when painting the top menu item, even when the items themselves are invisible while the menu is collapsed. The bug description might be better as: "Top menu items slow to paint/respond after displaying their sub-items (recursively) - the more the merrier".
Is this related to bug 820247?
(In reply to Vladan Djeric (:vladan) from comment #2) > Is this related to bug 820247? Yes, and bug 794246 (fix isn't complete apparently), bug 807581, bug 812904, and bug 830697. Some of those mention the degradation over time (due to new items displaying, possibly livemarks). Some mention it's fixed with HW accel off (it's faster without HW accel, but the basic issue still exists). This seems to describe the issue, though I'm not sure from this description if every hidden-but-previously-visible item should be repainted even when the entire menu is collapsed: (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from bug 794246 comment #14) > (In reply to Girish Sharma [:Optimizer] from comment #12) > > Even while using the default theme ? > > Yes. The problem is that there is not reliable way to determine that the > current theme isn't going to draw anything, so we have to do a lot of work > to set things up and ask the theme to draw even if it doesn't end up drawing > anything. Possible fix, at least if the issue raised by roc is hard to solve, is to treat the bookmark menu/toolbar similar to the history/recently-visited items, by destructing items, and repopulate the menu when required. It's not exactly the same though, because on the history menu the items are kept in mem and only deleted when new items replace them (when opening the menu). Using the same strategy for bookmarks will not solve this issue since many items will be kept in mem - The items need to be deleted when the popup closes IMO.
I can reproduce in Firefox18+ but not in Firefox17. Regression window(m-c) Good: http://hg.mozilla.org/mozilla-central/rev/360ab7771e27 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120820140803 Bad: http://hg.mozilla.org/mozilla-central/rev/f9a8fdb08193 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120821092158 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=360ab7771e27&tochange=f9a8fdb08193 Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/06ea22b8654c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120820013102 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/1f6cd8529ae9 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120820030143 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=06ea22b8654c&tochange=1f6cd8529ae9 And the problem is fixed in Aurora17.0a2 but not in mozilla-central. I.e., This problem still exist in Firefox18+. Fixed window(aurora channel) Bad: http://hg.mozilla.org/releases/mozilla-aurora/rev/7328213574c2 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120917160002 Fixed: http://hg.mozilla.org/releases/mozilla-aurora/rev/cbb5d4bf1617 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120917171701 Fixed pushlog: http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=7328213574c2&tochange=cbb5d4bf1617
FYI The slowness of the menu expanding resolves temporarily if I carry out the following procedures. 1. View > Toolbars > Customize... 2. Done
In local build Last Good: bc76f965c041 First Bad: 1f6cd8529ae9 Triggered by: 1f6cd8529ae9 Matt Woodrow — Bug 783449 - Move android specific NeedsPaint checks into the android widget implementation. r=roc
I've built 18.01 (cloned http://hg.mozilla.org/releases/mozilla-release/), and reverted the patch from bug 783449 (only at /widget/nsIWidget.h) and can confirm big responsiveness improvement on Windows (menu/bookmarks hovers, window resize - after expanding several bookmark folders with many subitems). With some usage patterns which I suspect are not uncommon (frequently using live/bookmarks toolbar or menu) Firefox 18.01 quickly becomes apparently unusable. I suggest to promote a fix for this, possibly upto FX18. With the reverted patch on FX18.01, on Windows: - Respond is noticeably faster at the relevant scenarios. - I cannot reproduce with STR at bug 783449 comment #0 (page info popup highlight remains). - I cannot reproduce with STR at bug 784117 comment #0 (awesomebar duplicate highlight). - I cannot reproduce with STR at bug 784137 comment #0 (awesomebar highlight remains). - I also tried collapsing menus while an item was highlighted, and then expand the menu again, and that item was not (erroneously) highlighted. Possible regressions to reverting the patch of bug 783449, which I didn't test: - bug 770000 (html5 video repaints too often). - bug 782980 (black boxes on bottom/right during resize). I did test this one on Windows, but the behavior seems the same before/after the revert (shows white area, as if the resize is async - though still slow as if it's sync). However, the bug is marked with platform=all. Maybe a promoted patch should only be applied for Windows?
Also could not reproduce with STR from bug bug 770000. However, my system is relatively fast (i7 3630qm), so the issue might be masked.
It is btw not just bookmarks menus. I also find it laggy when I mouse over tab strip overflow "List all tabs" menu, with maybe 50 menu items.
This is on a Mac.
any idea why the slowing while hovering mouse over bookmarks button does not occur if the buttons are set to the default size ?
(In reply to mayankleoboy1 from comment #12) > any idea why the slowing while hovering mouse over bookmarks button does not > occur if the buttons are set to the default size ? Because the bookmark placesView are reset after Customize Toolbar.
Thanks for the detailed analysis and regression range here. The issue is that each submenu is creating a new widget, and we then end up spending large amounts of time processing each of these widgets during paint. I think the right thing to do here is revert the original change, so that we skip over hidden or 0-sized widgets. I've also added an alternative fix for the original bug, which just invalidates the entirety of the popup when it becomes visible.
Attachment #704692 - Flags: review?(roc)
Attachment #704692 - Flags: review?(roc) → review+
Please nominate for Aurora and Beta after this has landed.
Assignee: nobody → matt.woodrow
Comment on attachment 704692 [details] [diff] [review] Revert, and invalidate the popup instead [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 783449 User impact if declined: Massive slow downs after a large number of submenus have been viewed. Testing completed (on m-c, etc.): Landed on m-c Risk to taking this patch (and alternatives if risky): Low risk. String or UUID changes made by this patch: None
Comment on attachment 704692 [details] [diff] [review] Revert, and invalidate the popup instead approving low risk fix for uplift. please land to beta immediately so it's ready for beta 4.
(In reply to Avi Halachmi (:avih) from comment #3) > (In reply to Vladan Djeric (:vladan) from comment #2) > > Is this related to bug 820247? > > Yes, and bug 794246 (fix isn't complete apparently), bug 807581, bug 812904, > and bug 830697. Should those (and the other bugs in the dependency tree) be marked fixed too now?
(In reply to :Gavin Sharp (use firstname.lastname@example.org for email) from comment #22) > Should those (and the other bugs in the dependency tree) be marked fixed too > now? Yes for bug 807581, bug 812904, bug 829563. Bug 820247: probably yes (mentions also slow tab close which I don't recall as a symptom of this bug, and relates the issue to number of open tabs - which I also don't recall as related). Bug 830697: I'd say yes, but will want the reporter to confirm. It doesn't states a problem but rather a solution (to a symptom of bug 794246), but it might be valid in its own right (delay widget border paint until its content is ready). The behavior which this bug wants to fix _should_ be gone now, though I couldn't reproduce it in the first place as well.
(In reply to Avi Halachmi (:avih) from comment #23) > Bug 830697: I'd say yes, but will want the reporter to confirm. Girish prefers to keep this one open.
Oops #2 (Bugzilla tweaks 1.12.1?? I never got close to these flags...). I couldn't restore the tracking flags.
Can you guys please do the honors for resetting the flags? I don't have the super powers necessary for the task! :-)
Saw the slowness in FF 19b3. Verified fixed in FF 19b4 Win 7 x64.
Depends on: 841308
Verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0b3
You need to log in before you can comment on or make changes to this bug.