User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/416.12 (KHTML, like Gecko) Safari/416.13 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20051111 Firefox/1.5 I attached a page showing the problem. Rendering the page, both on Mac OS X and on Windows, takes well over 20 seconds. On Firefox 1.0.7 the page is rendered instantaneous. I have isolated the issue: it is between the lines "START COMMENT OUT HERE" and "FINISH COMMENT OUT HERE" in the style sheet. Removing that part makes the page render instantaneous. (The layout is not as intended in that case, obviously.) Reproducible: Always Steps to Reproduce: 1. Extract the attached ZIP file 2. Load the page "example.html" in Firefox 1.5 Actual Results: Rendering the page takes over 20 seconds, both on Windows and on Mac OS X Expected Results: The page should render instantaneously, as it does in Firefox 1.0.7. I found a bug report that is probably not related to this, but I'm not sure, so I give you the bug ID: #319644. The reason that I set the severity to "major" is that this issue makes our CMS back-end unusable for Firefox 1.5 users.

Dupe of one of the many 'firefox slow tables' bugs?

I don't think so, because this is a non-table css only layout.

This changed between 24 and 25 Nov 2004. Checkins: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-11-24+06%3A00%3A00&maxdate=2004-11-25+09%3A00%3A00&cvsroot=%2Fcvsroot

nested divs and tables don't behave much different (performance wise) Martijn, can you figure this one out ?

Seems to be happening because of the floats and the use of lists.

With that regression range, it is very likely a regression from bug 209694.

This renders slow for me (lags for 5 sec or so), while it is still pretty fast in Mozilla1.7.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051217 Firefox/1.6a1 ID:2005121705 It loads normally, but it maximizes slowly :)

(In reply to comment #11) > Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051217 > Firefox/1.6a1 ID:2005121705 > > It loads normally, but it maximizes slowly :) > You need a slow computer like Martijn (600MHz) and I(850MHz) have Ria ;-) It takes ~5 secs to display and a repaint after window resize is ~10 secs.

I don't know if it's relevant, but please note that in my original example I could not get it to work by using other constructs than float, such as position absolute (from the right of the screen), position relative, etc. (All my attempts tried to retain the intended layout. Just displaying the 'other' div and the divs inside that as 'inline' makes it render instantaneous again, but destroys the layout.)

Using the latest trunk build is this still valid?

If you can direct me to a Mac OS X build of that version, I will be happy to test it... (In reply to comment #14) > Using the latest trunk build is this still valid?

(In reply to comment #14) > Using the latest trunk build is this still valid? Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060531 Minefield/3.0a1 ID:2006053104 [cairo] Going slower than ever with https://bugzilla.mozilla.org/attachment.cgi?id=205960 Rendering the test case takes 100% of CPU for 50 seconds running on Athlon XP 2800+! On the plus side, the test case displays incrementally. Even closing the tab that has the test case open takes about 10 seconds! Just tested the very same test case with Opera/9.00 (X11; Linux i686; U; en) and it has no problems with it.

I just wanted to point out that this bug has not been solved in Firefox 2.0 beta 1. Is the version field (currently '1.8 Branch') correct? Shouldn't this be 'trunk'?

This is almost certainly a regression from bug 209694. For what it's worth, a trunk build loads the "minimised testcase" in 17 seconds, while a reflow branch build does so in 6. Which is still way too much...

So for the minimized testcase, I tried logging the times through the |while (PR_TRUE)| loop in nsBlockFrame::ReflowBlock. We hit the beginning of that loop 44463 for 186 distinct blocks; an average of 250 times a block. 84 blocks got reflown 512 times. 3 blocks got reflown 256 times. 2 blocks got reflown 128 times. 3 blocks got reflown 64 times. 2 blocks got reflown 32 times. 3 blocks got reflown 16 times. 3 blocks got reflown 8 times. 3 blocks got reflown 4 times. 2 blocks got reflown 3 times. 4 blocks got reflown 2 times. 77 blocks got reflown 1 time. Sadly, the "reflow counts" option in the layout debugger seems broken. :( So not sure which blocks we're reflowing that much.

So yeah... removing the margin-collapse retry makes this page render beautifully. Which makes sense -- the page has a set of deeply nested blocks each of which ends up needing the retry. So the time taken is exponential in the depth of the nesting. :( So this looks like a different issue from bug 273293, and one we should really try to deal with for Gecko 1.9, imo.

After reflow branch has landed, I'll try to make the second-pass reflow more incremental so it doesn't have to reflow block children that haven't changed.

(In reply to comment #21) > After reflow branch has landed, I'll try to make the second-pass reflow more > incremental so it doesn't have to reflow block children that haven't changed. In case you haven't noticed, the reflow branch has landed ;)

This patch nails the performance problems completely, as far as I can tell. The idea is to abort reflow as soon as we detect that a second pass will be needed. I believe this reduces possible O(2^N) behaviour to O(N). However, this approach to aborting reflow is, at best, a huge hack. David, what do you think the right interface should be to allow Reflow() to abort this way? A magic rv, a new flag in nsHTMLReflowMetrics, or a new nsRelowStatus? And how specific should the abort signal be...

It seems to me that we don't need another flag or special return; we already have aState.mReflowState.mDiscoveredClearance to indicate this situation.

Comment on attachment 273031 [details] [diff] [review] Simplified version of proof of concept Mmmmm ... yeah!!!

Checked in.

Would be nice if this could be tested in some way.

We could just toss the testcase into reftests so that they'd fail if this regressed.

Ok, but I especially meant the performance part of the bug. I mean, I think it's important to not regress this again.

If we put a long enough version of the testcase into reftests, the testing scripts will kill it :) I think reftests automatically fail if they take more than 20 secs or something like that.

maybe regression by this check-in, see below. http://forums.mozillazine.org/viewtopic.php?p=2980171#2980171

I filed bug 389398 on the apparent regression.

Backed out due to the regression + freeze.

I really think this should be blocking, not just wanted....

This is pretty bad, and it blocks a blocker, so we'd better reevaluate it.

Here's a revised version of Eli's patch. The main change is that instead of returning from ReflowDirtyLines when we detect the clearance-change, we keep going, we just don't reflow any of the dirty lines (treating them as not dirty). This fixes the regression that patch caused, which I think was caused by SlideLine not being called so we forget that lines have moved. In general it's a more conservative fix because we execute mostly the same ReflowDirtyLines code in the "clearance changed" case as the normal case. It still fixes performance in the testcases mentioned in this bug nicely, though. The other change is to make CheckFloats not spew warnings when lines haven't had their float caches updated. If there are dirty lines, then we will reflow again so it's too early to spew warnings.

I don't know where "v7" came from...

P1 because it's moderately risky and fixes real pages

Comment on attachment 286522 [details] [diff] [review] updated patch v7 OK, r+sr=dbaron. Sorry for the delay.

checked in.