Closed Bug 240275 Opened 16 years ago Closed 16 years ago

Painting large tables is slow

Categories

(Core :: Web Painting, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: roc)

References

()

Details

(Keywords: perf)

Attachments

(1 file)

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.
Attached patch patchSplinter Review
Robert could you please run your pixel comparison on that. Thats the thing we
talked last week on irc. 
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 on attachment 146741 [details] [diff] [review]
patch

sounds like no risk no fun
Attachment #146741 - Flags: superreview?(roc)
Attachment #146741 - Flags: review?(roc)
Attachment #146741 - Flags: superreview?(roc)
Attachment #146741 - Flags: superreview+
Attachment #146741 - Flags: review?(roc)
Attachment #146741 - Flags: review+
fix checked in, net effect on tinderbox pageload: zero.
As usual. Now I understand why Chris did not touch these bugs ;-).
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
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.
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.  :(
Depends on: 241796
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.