226243 - Very slow loading of 2MB file with a quite big table with lots of colspans

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Wojciech Czarnowski]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031119
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031119

The indicated URL is 2 MB in size, mostly one big table. Loading it from local
drive takes more than 2 minutes. IE6 handles it in 10 seconds.


Reproducible: Always

Steps to Reproduce:
1. Download http://bazyl.rodos.com.pl/~wc/mozilla/test.zip and unpack it.
2. Open file test.htm in the browser.
3. Wait.


Actual Results:  
Loading of the file completed in more than 2 minutes.

Expected Results:  
Load the file in a few seconds.

Comment 1

[Jo Hermans]

bug 54542 I suppose

Comment 2

[Boris Zbarsky [:bzbarsky]]

It's not.  Profile info coming up.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Profile

The salient part is:

Total hit count: 29061
Count %Total  Function Name
26270	90.4	 nsCellMap::GetDataAt(nsTableCellMap&, int, int, int)
250   0.9     BasicTableLayoutStrategy::RowSort(int*, int*, int)

The main callers of GetDataAt are:

nsCellMap::GetEffectiveColSpan
nsCellMap::GetCellInfoAt
nsCellMap::RowIsSpannedInto

Comment 4

[Boris Zbarsky [:bzbarsky]]

I this case, I bet the problem is that we're looping over all columns getting
celldatas... and for some columns there is no data anywhere in the table, pretty
much (the page uses colspan="5" on corresponding cells in each row).  So since
the caller passes PR_TRUE for "zero-expand" we walk up each column looking for
celldata... and fail to find it, of course.

The end result is that pageload is O(N^2) in the number of table rows, since
each GetDataAt() call ends up being O(N) in the number of rows.

Is there a reason not to create the "dummy" celldatas that
nsCellMap::AdjustForZeroSpan creates to start with?  It'll use a bit more
memory, but pages using 0 for rowspan or colspan are rare, and pages with very
long tables are much more common.... and I'm tired of seeing GetDataAt as the
major contributor in every single "tables are slow" profile.  ;)

Comment 5

[Markus Hübner]

Maybe we should update the summary of this bug to a more 
meaningful/descriptive one?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Why?  It's perfectly meaningful -- the key is "lots of colpans" here.  If you
notice, the summary did get updated once already...

Comment 7

[Markus Hübner]

Sorry - that comment should have gone to bug 220937

Comment 8

[Bernd]

Boris could you please create a new profile now that jim nance has changed the
layout strategy and I did a few tweaks to the cellmap?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated profile

With a build pulled at "Sat Jul 17 14:00:13 CDT 2004" I have the following:

Total hit count: 27516
Count	%Total	 Function Name
25285	91.9	 nsCellMap::GetDataAt(nsTableCellMap&, int, int, int)
109	0.4	 nsCellMap::GetCellInfoAt(nsTableCellMap&, int, int, int*,
int*)
 82	0.3	 nsStyleContext::GetStyleData(nsStyleStructID)

So RowSort has been improved...

The main callers of GetDataAt are:

  17046 nsCellMap::GetEffectiveColSpan
  5308 nsCellMap::GetCellInfoAt
  2911 nsCellMap::RowIsSpannedInto

So doesn't look like much has changed here.  :(

Comment 10

[Bernd]

Attachment: first guess

Boris could you generate a new jprof with the patch just to get an indea where
the problem is

Comment 11

[mstuhu]

Recent Mozilla and Firefox versions still show this bug and it's really annoying
when you have to work all day with larger tables.

It would be really nice to see a performance increase in order to work more
efficient with Firefox.

Comment 12

[mstuhu]

If you want to experience the slowness of large table loading on a live website,
take a look here:
http://weightweenies.starbike.com/listings/components.php?type=mtbtyres

IE6 -> about 1 s
Latest Mozilla -> 6-7 seconds, even worser when scrolling while the table is
being drawed

Comment 13

[Martijn Wargers (dead)]

The url is gone.

Comment 14

[mstuhu]

(In reply to comment #13)
> The url is gone.

Works for me, try it again,

Comment 15

[Martijn Wargers (dead)]

I was talking about the bug url ( http://bazyl.rodos.com.pl/~wc/mozilla/test.htm
), not about http://weightweenies.starbike.com/listings/components.php?type=mtbtyres
That url I can see, but the issue you're seeing might be a different issue than
what this bug is about.
I suggest you file a new bug on that and report the bug number here.

Comment 16

[mstuhu]

> http://weightweenies.starbike.com/listings/components.php?type=mtbtyres
> That url I can see, but the issue you're seeing might be a different issue than
> what this bug is about.
> I suggest you file a new bug on that and report the bug number here.

What do you mean by different issue?
Which component is the problem related to?
I'm still suffering from the problem, that's why I'd like to file a new bug now.

Comment 17

[Martijn Wargers (dead)]

(In reply to comment #16)
> I'm still suffering from the problem, that's why I'd like to file a new bug
> now.
Pleas do file a new bug.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Original testcase is missing... but the analysis I did initially shows that bug 351942 should have fixed this.