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)
Tracking
()
RESOLVED
FIXED
People
(Reporter: pedro, Assigned: bernd_mozilla)
References
(Blocks 1 open bug, )
Details
(Keywords: hang, testcase)
Attachments
(3 files)
(deleted),
application/zip
|
Details | |
(deleted),
application/zip
|
Details | |
(deleted),
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
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.
Comment 1•20 years ago
|
||
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+
Comment 2•20 years ago
|
||
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
Comment 3•20 years ago
|
||
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+
Comment 4•20 years ago
|
||
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.
Comment 5•20 years ago
|
||
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
Comment 6•19 years ago
|
||
The testcase freezes current trunk builds still up.
Boris could you please make me a fresh profile once I landed the rowisspanned thing?
Comment 8•19 years ago
|
||
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)
Boris, this should improve the profile. Could you please test with it?
Comment 10•19 years ago
|
||
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. ;)
Assignee | ||
Comment 11•19 years ago
|
||
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 12•19 years ago
|
||
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+
Assignee | ||
Comment 13•19 years ago
|
||
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.
Assignee | ||
Comment 15•19 years ago
|
||
.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 16•19 years ago
|
||
Did this cause a regression with GMail ?
The "Labels" and "Invite a friend" boxes seem to take up the whole page (width-wise).
Comment 17•19 years ago
|
||
My build with this patch shows google just fine. Is there a bug for this regression?
Comment 18•19 years ago
|
||
(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.
Comment 19•19 years ago
|
||
*** Bug 337916 has been marked as a duplicate of this bug. ***
Comment 20•18 years ago
|
||
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)
Comment 21•18 years ago
|
||
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.
Comment 22•18 years ago
|
||
> 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.
Description
•