253001 - slow to load (20 seconds, CPU 90%), slow to scroll (10 seconds, CPU 90%)

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Brent Beach]

User-Agent:       Mozilla/5.0 (Windows; U; Win 9x 4.90; en-GB; rv:1.8a3) Gecko/20040724
Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-GB; rv:1.8a3) Gecko/20040724

Mozilla loads about half this page then gets very busy for about 20 seconds
(cannot start other tasks during this time). Finishes loading the page. Scroll
down causes Moz to get very busy again, for 10 to 20 seconds. 

Reproducible: Always
Steps to Reproduce:
1. Load the page
2.
3.

Actual Results:  
The spinning circle stops about half way through the load for about 20 seconds.
I loaded the latest MOZ, started the task monitor, did it again and saw the high
CPU usage during load and scrolling.

Expected Results:  
IE loads this page, a little slowly, but not as slowly as MOZ. During the IE
load, its status bar said "Building menu" so perhaps the alphabet across the top
of the screen is a large menu that takes some time to build. IE had no delays in
scrolling the screen, just in building it.

Comment 1

[Martijn Wargers (dead)]

There seems something wrong with the image inside the menus. It seems corrupt or
something like that.

Comment 2

[Boris Zbarsky [:bzbarsky]]

The pageload start pause is in fact the building of the menu.  I'm not sure how
we compare to IE (can't test IE), and I'm testing this in a build in which it
should be a lot faster anyway.  But the time is mostly going towards creating a
large number of nodes, setting style on all of them, etc, etc (with the usual
JS, XPConnect, and security manager overheads).

I'll profile the scrolling next.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Scrolling is a fun one.  I'll attach the full profile in a second, but here's
the summary:

Total hit count: 491632
Count %Total  Function Name
111590   22.7     nsRegion::InsertInPlace(nsRegion::RgnRect*, int)
104891   21.3     nsRegion::Copy(nsRegion const&)
83878   17.1     nsRegion::SetToElements(unsigned)
80136   16.3     RgnRectMemoryAllocator::Alloc()
55603   11.3     nsRegion::MoveInto(nsRegion&, nsRegion::RgnRect const*)
12825   2.6     nsRegion::SubRect(nsRegion::nsRectFast const&, nsRegion&,
nsRegion&) const
12077   2.5     _init
11441   2.3     RgnRectMemoryAllocator::Free(nsRegion::RgnRect*)
8991   1.8     __i686.get_pc_thunk.bx
3437   0.7     __libc_poll
2545   0.5     nsRegion::Optimize()
1882   0.4     nsRegion::Init()
1308   0.3     nsRegion::SubRegion(nsRegion const&, nsRegion&) const
121   0.0     ApplyZOrderStableSort(nsVoidArray&, nsVoidArray&, int, int)
101   0.0     nsViewManager::OptimizeDisplayList(nsVoidArray const*, nsRegion
const&, nsRect&, nsRegion&, int)

More to the point, 487375 of the 491632 hits (this is for scrolling two
down-arrow-keys worth down the page) are under nsViewManager::OptimizeDisplayList.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Well, aDisplayList->Count() is on the order of 6000 when OptimizeDisplayList is
called there....  

It looks like the page has thousands of elements with position, visibility,
overflow, and z-index set on them...

Comment 5

[Martijn Wargers (dead)]

Attachment: Zipped testcase

This is the output of that javascript menu.
I've stripped it from all tags and style properties that don't cause a
performance breakdown.
Another observation: when I remove - in this testcase - all instances of '
overflow: hidden;' or all instances of ' background-color: white;', the
performance seems to be all right.

Comment 6

[Martijn Wargers (dead)]

Removing all instances of ' position:absolute;' also seems to work.

Comment 7

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

Sounds like the opaque region is just getting way too complicated and making
region operations very slow.

An easy hack would be to abort OptimizeDisplayList when too many opaque views
have been detected.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Somewhat like this?

Comment 9

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

Comment on attachment 155730 [details] [diff] [review]
Somewhat like this?

Actually I think the thing to do is to just stop adding opaque views to the
opaque region when the opaque region gets beyond MAX_OPAQUE_VIEWS_TO_OPTIMIZE
(which should be called something more like MAX_OPAQUE_REGION_COMPLEXITY)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: That would be like so.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fixed for 1.8a3.

Comment 12

[Boris Zbarsky [:bzbarsky]]

This improved the "overall" Tdhtml metric by about 6%.  Looking at the specific
tests, all the win came from the two tests that move around 200 divs.  Before
this patch:

layers1:       10876  10658   9674,10808,10876,10820,11110
layers5:       7493   7554    7480,7577,7809,7411,7493

After this patch:

layers1:       5787   5781    5667,5795,5878,5780,5787
layers5:       5476   5500    5475,5591,5476,5483,5473

Comment 13

[jag (Peter Annema)]

+// with MAX_OPAQUE_REGION_COMPLEXITY rets in it, we stop adding views' rects to

"rets" should be "rects"