240275 - Painting large tables is slow

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Boris Zbarsky [:bzbarsky]]

The main problem is that nsTableRowGroupFrame::PaintChildren doesn't intersect
the dirty rect with the kids' rects (see
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableRowGroupFrame.cpp#259).
 I assume we want to skip the intersect check if and only if the row has a cell
spanning out of it (since then we need to paint that row if that cell intersects
the dirty rect), right?  Is there a fast way to detect this situation that we
could use here, bernd?

As things stand, scrolling the page in the URL field (which you may not even be
able to load without applying the patch in bug 54542) is _very_ painful.

Comment 1

[Bernd]

Attachment: patch

Comment 2

[Bernd]

Robert could you please run your pixel comparison on that. Thats the thing we
talked last week on irc. 

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I ran the tests. There were no changes other than sporadic failures, which are
almost certainly due to my testing infrastructure rather than your patch.

My tester isn't necessarily that effective on this kind of patch. Usually it
captures right after the first and only reflow of the entire page, which usually
repaints everything, so any frame which overlaps the visible area of the page
will paint itself even if its overflow area is incorrect or the visibility
checks are coded incorrectly.

Still, it's good that we didn't find any obvious bugs :-)

Comment 4

[Bernd]

Comment on attachment 146741 [details] [diff] [review]
patch

sounds like no risk no fun

Comment 5

[Bernd]

fix checked in, net effect on tinderbox pageload: zero.
As usual. Now I understand why Chris did not touch these bugs ;-).

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Like I said, on most pages there's just one big paint that repaints all the
frames that are visible on the page. The win from this patch, if there is one,
is when you scroll or otherwise repaint just a small part of the page many many
times.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Bug 241796 filed on the remaining issue I see in the profile with that patch
applied.

Bernd, roc is right -- this was not a page layout issue but an interactive
performance issue, and tinderbox has absolutely no test pages with a large table
on them.  So it's no good whatsoever for testing any of the large table
issues... certainly not ones that require user interaction like this one.  :(