Closed Bug 271789 Opened 20 years ago Closed 19 years ago

Firefox hangs for 30 seconds or more when accessing this URL

Categories

(Core :: Layout: Tables, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: pedro, Assigned: bernd_mozilla)

References

(Blocks 1 open bug, )

Details

(Keywords: hang, testcase)

Attachments

(3 files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041109 Firefox/1.0 (MOOX M2) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041109 Firefox/1.0 (MOOX M2) When accessing this page, Firefox 1.0 (on WinXP) hangs for 30 seconds or more (on an Athlon XP 2000+, 1 GB of RAM, updated drivers and everything). My first thought is that it wasn't recoverable, but I waited and, after that time, I had control of the browser again. It doesn't crash the browser, but, of course, if I try to close the Firefox window while it's "hung", Windows gives the "this program is not responding" message. Firefox has no problem with other sites, and I browse a lot. Reproducible: Always Steps to Reproduce: 1. Go to http://www.playagain.net 2. On the left frame, click on "M.A.M.E." 3. On the main frame, click on "MAME 0.89" (I think the link may be updated as future M.A.M.E. versions are released) Actual Results: Part of the list appears, but then Firefox (all tabs & windows) "hangs" for 30 seconds or more. After that, the browser begins to work again, including that particular URL (it's a big list of links). Expected Results: Even if the list takes a while to download, the browser should not hang.
I'm seeing this also with: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041125 Firefox/0.9.1+
That page contains a large table, so this may very likely be a less severe case of bug 148338. Happens in Moz1.7 as well.
Component: General → Layout: Tables
Product: Firefox → Core
Version: unspecified → Trunk
I can confirm this behaviour aswell in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041125 Firefox/0.9.1+
Attached file Testcase (deleted) —
The first table row has an extra table cell. This table cell had also rowspan="5314", but that doesn't seem to matter in rendering speed. Removing the extra table cell drastically improves rendering speed.
Keywords: testcase
Keywords: hang
Usual deal: Total hit count: 217827 Count %Total Function Name 174214 80.0 nsCellMap::GetDataAt(nsTableCellMap&, int, int, int) The main caller of this is nsCellMap::GetEffectiveColSpan, though nsCellMap::RowIsSpannedInto and nsCellMap::ColHasSpanningCells etc also contribute. Note that I did the profile with a Dec 2 build, but I don't think anything has gone in since then that would affect this (the testcase does not set a rowspan or colspan).
Assignee: firefox → nobody
Blocks: 234240
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: firefox.general → core.layout.tables
The testcase freezes current trunk builds still up.
Boris could you please make me a fresh profile once I landed the rowisspanned thing?
Depends on: 313295
Highlights: Total hit count: 266376 Count %Total Function Name 218996 82.2 nsCellMap::GetDataAt(nsTableCellMap&, int, int, int) nsCellMap::GetDataAt called from: 163727 nsCellMap::GetEffectiveColSpan(nsTableCellMap&, int, int, int&) 37735 nsCellMap::RowIsSpannedInto(nsTableCellMap&, int, int) 16524 nsCellMap::ColHasSpanningCells(nsTableCellMap&, int) nsCellMap::GetEffectiveColSpan is called via nsTableFrame::GetEffectiveColSpan, which is called by: 55826 CalcAvailWidth(nsTableFrame&, int, float, nsTableCellFrame&, int, int&, int&) 48339 nsTableCellFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) 44030 nsTableRowFrame::ReflowChildren(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, nsTableFrame&, unsigned&, int) 15939 nsTableFrame::CellChangedWidth(nsTableCellFrame const&, int, int, int)
Attached patch speedup (deleted) — Splinter Review
Boris, this should improve the profile. Could you please test with it?
With that patch, loading this page takes about 2-3 seconds over here and the profile says: Total hit count: 42278 Count %Total Function Name 8437 20.0 __libc_poll 1021 2.4 nsStyleContext::GetStyleData(nsStyleStructID) 860 2.0 __i686.get_pc_thunk.bx 765 1.8 nsRuleNode::GetStyleData(nsStyleStructID, nsStyleContext*, int) 607 1.4 SelectorMatches(RuleProcessorData&, nsCSSSelector*, int, nsIAtom*, signed char) 474 1.1 nsCellMap::GetDataAt(nsTableCellMap&, int, int, int) 446 1.1 nsHTMLReflowState::InitConstraints(nsPresContext*, int, int, nsMargin*, nsMargin*) 443 1.0 __libc_calloc So looks good to me. ;)
Comment on attachment 201947 [details] [diff] [review] speedup While the patch has only 5 meaningfull lines, its a rather drastic change. I thought of it for a long time and wondered how easy it is to implement. So for normal cells nothing changes. Things change for the cell holes. Before the patch the hole initiated a search for a orinating cell with zero span above or to the left. We searched as long as we did hit cell holes. What we do now is we say once the search does not give a positive result, there is no zero span above and to the left, its a dead cell. If now another hole initiates a search and hits the dead cell it will stop searching in this direction. Usually we walk from top left, so basically we will typicall never lookup more than one cell hole in each direction instead of searching the whole table. The bad thing is that the zero spans arent well tested and will take ages till regressions from this will surface.
Attachment #201947 - Flags: superreview?(bzbarsky)
Attachment #201947 - Flags: review?(bzbarsky)
Comment on attachment 201947 [details] [diff] [review] speedup So the idea is that the next time we'll hit this cellData and bail out, right? What impact will this have on the size of the cellmap in typical cases? I'm assuming it won't be much except in pathological cases like this, right?
Attachment #201947 - Flags: superreview?(bzbarsky)
Attachment #201947 - Flags: superreview+
Attachment #201947 - Flags: review?(bzbarsky)
Attachment #201947 - Flags: review+
Under normal conditions where each cellmap entry corresponds to a originating cell or is spanned by a row- or col span we will not increase the cellmap. If however due to too many col tags, or row- colspans hooles are created then we will fill the hools.
fix checked in
Assignee: nobody → bernd_mozilla
.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Did this cause a regression with GMail ? The "Labels" and "Invite a friend" boxes seem to take up the whole page (width-wise).
My build with this patch shows google just fine. Is there a bug for this regression?
(In reply to comment #17) > My build with this patch shows google just fine. Is there a bug for this > regression? > I've just filed bug 316040 and CC'ed you to it.
Depends on: 317148
Depends on: 328339
*** Bug 337916 has been marked as a duplicate of this bug. ***
Is this bug fixed ? I'm using Mozilla/5.0 (X11; U; Linux ppc; fr; rv:1.8.1.1) Gecko/20061205 Iceweasel/2.0.0.1 and can very reproduce with http://ompf.org/stuff/embroider/sample/radius.html (And btw, the more i load this page, the more memory firefox uses)
I have opened https://bugzilla.mozilla.org/show_bug.cgi?id=371718 with a better description. Again, i don't think the problem is about optimizing rendering, it's about making rendering not block the UI.
> Is this bug fixed ? Yes, but it was fixed in November 2005 and you're using a build with a rendering engine from August 2005 with security fixes since then. Then again, you're testing on a different page from the one this bug was filed on, so.... > it's about making rendering not block the UI. There are existing bugs on that.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: