Closed Bug 547128 Opened 15 years ago Closed 15 years ago

slow menu item/bookmark folder opening/navigation behavior

Categories

(Firefox :: Theme, defect, P2)

Product:
Firefox
Component:
Theme
Platform:
x86
Windows XP
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- final+

People

(Reporter: xtc4uall, Unassigned)

References

Details

(Keywords: perf, regression)

there's slow/sluggish responding behavior on opening menu items and bookmark folders on the bookmark toolbar what gets worse by time. see also bug 544999 comment 14. regrange: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a73612940488&tochange=410009589ea9 (i.e. Bug 546098 and/or Bug 545842) i'm not sure if this issue is covered by Bug 546262 already since i see no time dependency there.
I don't see how the patches in the range could cause this. Sounds like a platform bug if they do.
Keywords: qawanted
Is there an easy way for the non-dev to profile it?
Maybe related to bug 394283 ?
guess no, since this issue is a recent regression for sure. i did some local/manual hacking by deleting the references to the "winstripe-keyhole-forward-mask.svg" file, but the issue remains. maybe it's causes by the added/changed CSS rulesets by an exposed/underlying layout/parsing bug?! moreover i'd be happy if someone could verify/falsify my regression range by testing builds from http://hourly-archive.localgho.st/hourly-archive2/mozilla-central-win32/. you have to be sure that your bookmark toolbar contains several entries and (maybe) using winxp.
(In reply to comment #4) > moreover i'd be happy if someone could verify/falsify my regression range by I get the following regression range (pretty much the same as yours): http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a73612940488&tochange=1b944ebb5ca6 Which includes three changesets; however, since changeset 1b944ebb5ca6 simply fixed a typo in a comment, the responsible bugs have to be Bug 546098 and/or Bug 545842, as you identified Please add blocking of those two bugs and let Dao sort it out.
@Dao: would you, or anyone, be able to create a try-server build with Bug 545842 backed out?
Folks, it *never* happens for me if I do not open the Bookmarks menu. Once I start navigating the bookmarks menu it gets progressively worse and then affects all other menus (File, Edit, View, etc.). Let me add that I recently blew away my entire Firefox profile and started from scratch and only have a few add-ons installed (the same ones I had before I wiped my Firefox profile). I've also vacuumed my Places database to be sure this isn't the cause. BUILD: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a3pre) Gecko/20100302 Minefield/3.7a3pre (.NET CLR 3.5.30729) ID:20100302134520 ~B
Component: Theme → Bookmarks & History
QA Contact: theme → bookmarks
Not sure why component is set to bookmarks. This regression has nothing to do with bookmarks. It's a theme regression caused by either Bug 546098 or Bug 545842. Maybe accessing bookmarks menu exacerbates it more, because of the relative number of objects, but it is the wrong place to focus. Setting components to bookmarks is wholly misleading. Both XtC4UaLL and I independently came up with the same regression range, pointing to the exact same likely causes.
And in case anyone is wondering, I was able to identify the regression range using primarily the Tools and File menus. So this bug is not shackled to either Bookmarks or History. It affects ALL the menus.
One more thing. Since Bug 546098 and Bug 545842 rely heavily on CSS, this regression probably points to a performance issue with some aspect of Gecko's CSS implementation.
Component: Bookmarks & History → General
Product: Firefox → Core
QA Contact: bookmarks → general
So... for someone who can reproduce, can you narrow it down to which of bug 546098 and bug 545842 is responsible? Some things that are worth paying particular attention to: 1) Bug 545842 makes some boxes have different border-radii for different corners. That may be slower than having the same radius across the board. 2) Bug 546098 tosses in some opacity styles. This may slow down painting depending on what those styles are applied to.
blocking2.0: --- → ?
(In reply to comment #11) > 1) Bug 545842 makes some boxes have different border-radii for different > corners. That may be slower than having the same radius across the board. We don't have toolbar items that would be affected by this. The forward button does have different radii, but bug 545842 didn't change this.
(In reply to comment #11) > So... for someone who can reproduce, can you narrow it down to which of bug > 546098 and bug 545842 is responsible? If someone creates [tryserver] builds with either backed out -- especially bug 545842 -- I could tell you which is the culprit. Not only do I not know how to create the builds myself, I do not have sufficient computing resources for such a task. The original theme changes, brought about by bug 544999, did not affect performance, which is why I highly suspect bug 545842. But as I've said, make available the requested tryserver build and I'll confirm it.
Thanks, Boris. I definitely see the regression with the tryserver build: http://hg.mozilla.org/try/rev/1a0febbccbcc
OK. Are you willing to try editing those theme files to see which styles might be responsible? Note that you don't have to build yourself to do that; you should be able to quit the browser, edit the files on disk, then restart the browser... Might have to unzip the jar, edit, then rezip depending on how the files got shipped.
OK. Where do I find the particular files? I'm especially suspicious of this block: +.toolbarbutton-menubutton-button:not([disabled="true"]):not(:active):hover, +:not([open="true"]):not(:active):hover > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]), .toolbarbutton-1:not([disabled="true"]):not([checked="true"]):not(:active):hover { background-color: rgba(255,255,255,.6); border-color: rgba(60%,60%,60%,.3) rgba(60%,60%,60%,.45) rgba(60%,60%,60%,.75); -moz-box-shadow: 0 1px 0 rgba(0,0,0,.04), 0 0 6px -1px white, 0 0 6px -2px highlight, 0 0 5px -2px highlight, 0 4px 4px -3px white inset, 0 -4px 4px -3px white inset, 0 -3px 12px -5px highlight inset; -moz-transition: background-color .6s; } +.toolbarbutton-menubutton-button:not([disabled="true"]):hover:active, +:hover:active > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]), +[open="true"] > .toolbarbutton-menubutton-dropmarker, .toolbarbutton-1:not([disabled="true"]):hover:active, .toolbarbutton-1[checked="true"], .toolbarbutton-1[open="true"] { background-color: transparent; border-color: rgba(0,0,0,.5) rgba(0,0,0,.4) rgba(0,0,0,.4); -moz-box-shadow: 0 0 9px rgba(0,0,0,.4) inset, 0 0 3px rgba(0,0,0,.4) inset, 0 1px 0 rgba(255,255,255,.4); text-shadow: none; }
chrome/browser.jar!/skin/classic/browser/browser.css if you're on XP. chrome/browser.jar!/skin/classic/aero/browser/browser.css if you're on Vista/Win7.
Thanks. Located the block I mentioned. Is there anything from the patch that, if commented out, must be replaced with something else in order for things to function properly?
Depends on what "function properly" means. Every single line fulfills a purpose, but you should be able to start the browser without them.
The culprit is the block I mentioned in comment 17. If I comment out either the top half or bottom half of the listed block, the regression remains. When I comment out the whole listed block, the regression disappears.
If you need me to narrow it down further, let me know. I wasn't sure what to separate out.
(In reply to comment #21) i confirm this. furthermore i tried to track down the issue to one single css style as the cause by deleting the styles in several combinations (esp. the shadow and transition styles), but wasn't able to find the culprit :-/.
Well, hold on. If you remove all the declarations from the two rules listed in comment 18, does the problem go away?
Comment 18? If you mean comment 17, which ones are the declarations (I'm a CSS ub3r n00b)?
Er, I did mean comment 17. The declarations are the things coming after the '{', before the '}', and separated by ';'.
With the declarations commented out, the regression still exists. Therefore, these are the problem lines: +.toolbarbutton-menubutton-button:not([disabled="true"]):not(:active):hover, +:not([open="true"]):not(:active):hover > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]), .toolbarbutton-1:not([disabled="true"]):not([checked="true"]):not(:active):hover { } +.toolbarbutton-menubutton-button:not([disabled="true"]):hover:active, +:hover:active > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]), +[open="true"] > .toolbarbutton-menubutton-dropmarker, .toolbarbutton-1:not([disabled="true"]):hover:active, .toolbarbutton-1[checked="true"], .toolbarbutton-1[open="true"] { }
OK, now we're getting somewhere! If you remove the .toolbarbutton-menubutton-button:not([disabled="true"]):not(:active):hover selector from that first rule, does the problem go away? Similar for the other selectors involved.
I don't get it. If I remove only the line you've indicated, wouldn't I be left with something invalid?
As long as you also remove the comma after it, no. There are three selectors in that first rule, separated by commas. Removing any one of them (and one of the commas as needed) will leave the rule just fine. That said, now that I read those selectors again, the key ones seem to be: :not([open="true"]):not(:active):hover > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]) (first rule) and [open="true"] > .toolbarbutton-menubutton-dropmarker and :hover:active > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]) (second rule). What those selectors will do is reresolve style a lot on hover changes, active changes, and "open" attribute changes. I have no idea why such restyling would cause a performance problem that gets worse as time goes on, but that could at least be a place to start investigating. Can you check which, if any, of those three selectors cause the problem to happen?
For the first one, removing the following solves the problem: :not([open="true"]):not(:active):hover > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]) And for the second one it is: :hover:active > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]) If these shouldn't ordinarily have caused problems, maybe this exposes a performance issue with Gecko's CSS parser.
So if you remove either one of those two selectors the problem disappears? Or does it only disappear if you remove them both? Those selectors can cause a bunch of work to be done on every hover change on something that has no open="true" on it (for the former) or any hover + active change (for the latter). But that bunch of work shouldn't get slower and slower as time goes on; if it does, then there's a problem. Comment 0 seems to imply that it does. Is that what you're seeing too?
(In reply to comment #32) > So if you remove either one of those two selectors the problem disappears? Or > does it only disappear if you remove them both? Both must be removed. > Comment 0 seems to imply that it does. Is that what you're seeing too? Yes. Response gets slower with time, until the browser is restarted.
OK, excellent. Both needing to be removed makes some sense (though I wonder why :active is matching stuff here). I guess I should set up a Windows build to see what's going on here.... With those rules empty as in comment 27, are there any more transitions hanging around?
Component: General → Style System (CSS)
QA Contact: general → style-system
(In reply to comment #34) > With those rules empty as in comment 27, are there any more transitions hanging > around? I don't understand.
I'm still convinced this bug and bug 394283 are somewhat related. What I'm seeing is that opening several menupopups slows other things down. Things getting slower the more different menupopups were opened. Windows Spy++ tells that every menupopup leaves to a "MozillaDropShadowWindowClass" created. (BTW: these windows are never destroyed) Setting and resetting the "hidden" attribute on the navigator-toolbox will reset to normal speed. ("MozillaDropShadowWindowClass" windows are destroyed)
> I don't understand. It was mostly a question for Dão, but I'm basically asking whether there are any "-moz-transition: ...." rules hanging around. Jochen, could be! If you comment out the rules IU has been working with, do you see fewer "MozillaDropShadowWindowClass" windows created?
The only transitions are the one for the first part of comment 27 and another one for .toolbarbutton-1[checked="true"]:not(:active):hover.
(In reply to comment #37) > Jochen, could be! If you comment out the rules IU has been working with, do > you see fewer "MozillaDropShadowWindowClass" windows created? No.
(In reply to comment #37) > Jochen, could be! If you comment out the rules IU has been working with, do > you see fewer "MozillaDropShadowWindowClass" windows created? @Boris: Given what Jochen says about Firefox creating MozillaDropShadowWindowClass windows and not destroying them, if that is in fact the root cause, this bug would not have to cause the creation of additional MozillaDropShadowWindowClass windows to experience the increasing slowdowns. [And here my ignorance shows] So if those styles, which are the subject of this bug, are being applied to each of those [leaking] MozillaDropShadowWindowClass windows, it would make sense that things would get slower.
If everything is working correctly, I would expect one MozillaDropShadowWindowClass window to be lazily created for each menu when it's first opened and then cached after that. So that part sounds fine. In particular, sounds like those windows aren't obviously related to the rules above...
(In reply to comment #41) > If everything is working correctly, I would expect one > MozillaDropShadowWindowClass window to be lazily created for each menu when > it's first opened and then cached after that. So that part sounds fine. In > particular, sounds like those windows aren't obviously related to the rules > above... Sure. And I don't want to insist on the menupopup theory. I have tried two things after several menupopups were opened: 1. Setting and resetting the "hidden" attribute on the navigator-toolbox or 2. Setting and resetting the "hidden" attribute on every single menupopup Both will bring back normal speed for me. Just reporting my observations.
Jochen, I appreciate the observations. They're certainly helping narrow this down. So to make sure I'm clear on the situation, let's say we only ever open one menu (e.g. the File menu). Are the following statements true? 1) With the rules from comment 27, every time you open the menu it gets slower and slower. 2) Without those rules that effect does not exist. 3) Causing a frame reconstruct on that one menupopup (by setting and resetting the "hidden" attribute, for example; I assume you wait after setting and before resetting) gets performance to be fast again. 4) After the frame reconstruct from step 3, doing step 1 again shows the problem again.
(In reply to comment #43) > So to make sure I'm clear on the situation, let's say we only ever open one > menu (e.g. the File menu). Are the following statements true? I don't see a slowdown with just one menu involved. I can only see a slowdown with several different menus involved. The more menus involved the slower. With that in mind I can answer: 1) NO 2) Correct 3) Correct. (Yes, I'm using a timeout) 4) NO for single menu, YES for several menus I can also see the slowdown in Fx 3.6 after applying the two selectors without any rules.
Boris: If you have a Windows system, I have available a near copy of my main Firefox profile, which more easily shows the slow-down (a lag between when the menu is clicked to when it displays). With a new profile, the slow-down is initially difficult to notice, because the menu appears near instantaneously, so you need a system slow enough (I'm on a 933 MHz P3); however, once you navigate more menus, open and navigate the Bookmarks sidebar, etc., the slow-down becomes a bit more perceptible. As time goes on and more things are done with the browser (say an hour or more later), the slow-down becomes more evident. Let me know if the profile would be of any use to you and I'll finish scrubbing it then find a way to get it to you.
(In reply to comment #45) > Boris: If you have a Windows system... Just tested: I can see the same behaviour with Fx 3.5 on Ubuntu 9.04 after applying the two selectors without any rules.
More specifically: if you have an XP system. The reason being is the profile I have is for Firefox on an XP system.
Jochen: how did you apply those selectors on ubuntu? Where did you put them? IU, I was under the impression that the slowdown accumulated if you just opened and closed the menus over and over again without doing anything else. Is that not the case? I have a Windows 7 system, but not XP.
(In reply to comment #48) > Jochen: how did you apply those selectors on ubuntu? Where did you put them? First I was abusing one of my extensions, after that I simply put it into userChrome.css
(In reply to comment #48) > IU, I was under the impression that the slowdown accumulated if you just opened > and closed the menus over and over again without doing anything else. Is that > not the case? The problem immediately exists with the selectors applied; however, the increasing degradation, I believe, is the accumulative effect of various menus being accessed over time. Here's one way I found to quickly show the problem: 1. Click the menus (i.e. File, Edit, View, ... Help) and notice the speed with which they open (slower than without the problem selectors, but not easily noticeable) 2. Click Bookmarks menu and: a) for each entry that has a submenu, navigate to the child menu and run your mouse pointer up and down the bookmark entries. b) Repeat a) for all of the submenu-containing entries in the bookmark menu. 3. Once you're done with Step 2, Repeat Step 1. Notice how much slower the menus now respond. 4. Comment out the problem selections of comment 27 (or comment 31) and repeat Steps 1 to 3. Notice the difference.
I should have added a Step 0: Restart Firefox.
i removed 2 css selectors of comment 31, then the slowdown issue was gone too. Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a3pre) Gecko/20100304 Minefield/3.7a3pre ID:20100304040444
In addition, the problem seems to be solved when I avoid using UNIVERSAL RULES as follows. .toolbarbutton-menubutton-button:not([disabled="true"]):not(:active):hover, -:not([open="true"]):not(:active):hover > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]), +.toolbarbutton-1:not([open="true"]):not(:active):hover > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]), .toolbarbutton-1:not([disabled="true"]):not([checked="true"]):not(:active):hover { background-color: hsla(190,60%,70%,.5); border-color: hsla(190,50%,65%,.8) hsla(190,50%,50%,.8) hsla(190,50%,40%,.8); -moz-box-shadow: 0 0 0 1px rgba(255,255,255,.3) inset, 0 0 0 2px rgba(255,255,255,.1) inset, 0 0 5px hsl(190,90%,80%), 0 1px 0 rgba(0,0,0,.1); -moz-transition: background-color .5s ease-in; } .toolbarbutton-menubutton-button:not([disabled="true"]):hover:active, -:hover:active > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]), -[open="true"] > .toolbarbutton-menubutton-dropmarker, +.toolbarbutton-1:hover:active > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]), +.toolbarbutton-1[open="true"] > .toolbarbutton-menubutton-dropmarker, .toolbarbutton-1:not([disabled="true"]):hover:active, .toolbarbutton-1[checked="true"], .toolbarbutton-1[open="true"] { background-color: transparent; border-color: rgba(0,0,0,.5) rgba(0,0,0,.4) rgba(0,0,0,.4); -moz-box-shadow: 0 0 9px rgba(0,0,0,.4) inset, 0 0 3px rgba(0,0,0,.4) inset, 0 1px 0 rgba(255,255,255,.4); text-shadow: none; } CSSセレクタをより詳細に規定することで
Sorry, last japanese sentence in comment #53 is garbage.
Right, avoiding the universal selectors will make those rules not affect things except under nodes with the toolbarbutton-1 class. Not sure whether that's what the intent of the rules is, of course.
I made the changes Alice0775 White suggested above and I get a considerable improvement in File, Edit, View menu speed, especially for Bookmarks. It hasn't been this fast for me in awhile since maybe FF 2. Hope we can get a fix for this ASAP. This is especially impactful for those of us with deeply nested bookmark folders. ~B
The selector can be made more specific, although toolbarbutton-1 isn't the right class. Should I just do this or do you want to further investigate the slowness?
You should go ahead and do it if you can. I can investigate using userChrome.css per comment 49.
Blocks: 545842
fwiw, r=me on that change assuming that limiting by tag name is correct. We want it no matter what, since it leads to obviously faster code.
Before we close this one (if we consider the latest patch to be enough), we might want to look for other places where the same kind of "optimization" can be done. We might want to file an additional bug on that.
(In reply to comment #59) > http://hg.mozilla.org/mozilla-central/rev/ae8f4006916b Fix confirmed with: http://hg.mozilla.org/mozilla-central/rev/32383f6674e6 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a3pre) Gecko/20100305 Minefield/3.7a3pre ID:20100305073910
(In reply to comment #58) > You should go ahead and do it if you can. I can investigate using > userChrome.css per comment 49. I assume you're going to continue investigating the underlying issue; or is this going to be dropped and we have to go through this all over again when again it shows up?
> I assume you're going to continue investigating the underlying issue That's what I said in the comment you quoted, no? Now can you drop the passive-aggressive stuff?
(In reply to comment #64) > That's what I said in the comment you quoted, no? Now can you drop the > passive-aggressive stuff? I actually wasn't trying to be mean when I wrote that. Nor did I have ill intent. Sorry that it came off that way. All I was trying to do was make the point that, although this specific issue is resolved, if investigation ceased, this may rear its head again.
Fast with and without selectors: Gecko/20061207 Minefield/3.0a1 Gecko/20061207 SeaMonkey/1.5a Slow with selectors, fast without selectors: Gecko/20061208 Minefield/3.0a1 Gecko/20061208 SeaMonkey/1.5a Tested with fresh profiles, the 2 selectors inside userChrome.css and a bookmarks.html with several hundred bookmarks organized in appr. 30 subfolders.
Presumably the reflow branch landing.
OK. So I have no idea why reflow branch is involved yet, but it's pretty clear what one of the issues is. The reason comment 45 says you need to open/close some menus is that menupopup content (especially the frames) are generated lazily. They're cached after that. So once you open that long bookmark menu, all those nodes and frames are hanging around after that point. So once you've poked at your menus for a bit, you do all the lazy generation. Then you try to open a menu. You click on the menu. Every ancestor of the menu starts matching ":hover:active" and stops matching ":not([open="true"]):not(:active):hover" (which they started matching when you moused over the menubar). So we restyle all descendants of every ancestor. In particular, we restyle all the menus, their submenus, etc. You can test this easily; adding the tag selector "noSuchTag" to the selectors in question makes things fast, while adding "menubar" leaves them still slow, just not quite as slow. You can also test this by opening the menus via the keyboard shortcuts instead using the mouse. That should be fast even with those selectors. Now we happen to restyle the menus more than once; in particular we restyle them once each for the window, toolbar, toolbaritem, and menubar. Fixing that is one of the things bug 479655 will do. Then we will only restyle once (as if "menubar" were in the selectors). But past that, this is just a case where it happens to be easy to write a selector that's hard to optimize given a large DOM.... and our chrome using such a selector (bad, that). If it were not for the reflow branch connection, I'd say the checkin in comment 59 is the right fix for this bug. But I'd like to understand what, if anything, reflow branch has to do with this first.
Depends on: 479655
Maybe we should mark this FIXED by the checkin in comment #59, for tracking purposes?
Status: NEW → RESOLVED
Closed: 15 years ago
Component: Style System (CSS) → Theme
Product: Core → Firefox
QA Contact: style-system → theme
Resolution: --- → FIXED
blocking2.0: ? → ---
Yeah, I'm still planning to investigate but I'll file a separate bug as needed for it.
blocking2.0: --- → final+
Priority: -- → P2
I did look into this some, and I can't figure out why reflow branch matters here. Given that we want to rewrite the XUL code to be sane anyway, I'm going to stop worrying about this.
You need to log in before you can comment on or make changes to this bug.