137577 - scrollTo() on an incomplete dynamic document is slow

Status: RESOLVED WORKSFORME

(Core :: Layout, defect, P3)

Description

[Alan Cox]

A frame based irc client hack shows this problem very well. It uses an HTML
frameset, one frame of which is multi-part-replace of text-html. The cgi adds
text to the display line by line until it reaches a certain size then chops it in
half and does a new multipart-replace

Because the true size of the data is going to exceed the available window space
and the mozilla default is to keep the top of the document viewable each time a
line or group of lines is displayed a javascript scrollTo(0, 60000); is issued
to keep bottom tracking.

I've straced this and its just zillions of gettimeofday calls as if it was
spinning on a lock. With a static document I can't duplicate the problem.

Comment 1

[Alan Cox]

Dumped the entire session. It seems that unlike "the other browser" the mozilla
scrollTo is quite slow. The cgi was dumping many unneeded scrollTo calls and
that was having a bad effect to say the least.

So its merely "scrolLTo is slow"

Comment 2

[Christine Hoffman]

Changing QA contact

Comment 3

[Boris Zbarsky [:bzbarsky]]

Alan, any chance of a publicly available testcase that could be profiled?

Comment 4

[Kai Lahmann (is there, where MNG is)]

mail lost on spam?

Comment 5

[Alan Cox]

Sorry I missed the request

Just generate a document that contains a lot of ScrollTo commands as it renders.
The cgi was effectively doing

Line of text
<javascript ... scrollto(0,10000)  [ 4 or 5 repeats ]
line of text

continually

Comment 6

[Kai Lahmann (is there, where MNG is)]

so it can stay at WFM?

Comment 7

[Alan Cox]

It works but its slow. The application, even with the number of scrollto's
reduced by sensible coding is sluggish in mozilla and snappy in Internet Explorer

Comment 8

[Alan Cox]

Attachment: One way to demonstrate the problem

Slightly extreme example of the poor performance

Comment 9

[Kai Lahmann (is there, where MNG is)]

matic: could you take look, if this is covered by another bug?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: gzipped HTML jprof output for that testcase

If you decide to run the testcase, you need to s/scrollto/scrollTo/ on it.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Looking at that profil, the problem seems to be about what you'd expect -- all
those FlushPendingNotifications() calls are expensive.  We're reflowing all the
time, with none of the usual batching optimizations kicking in...  The top of
the flat profile looks like:

Total hit count: 5552
Count %Total  Function Name
656   11.8     nsBlockFrame::ReflowDirtyLines(nsBlockReflowState &)
628   11.3     nsFrameList::LastChild(void) const
579   10.4     nsBlockFrame::BuildFloaterList(void)
513   9.2     nsLineBox::GetCombinedArea(nsRect *)
409   7.4     UndisplayedMap::AppendNodeFor(UndisplayedNode *, nsIContent *)
139   2.5     nsBlockReflowState::RecoverStateFrom(nsLineList_iterator, int)
135   2.4     nsBlockFrame::ComputeFinalSize(nsHTMLReflowState &,
nsBlockReflowState &, nsHTMLReflowMetrics &)
 93   1.7     nsBlockFrame::PaintChildren(nsIPresContext *, nsIRenderingContext
&, nsRect &, nsFramePaintLayer, unsigned int)

The second entry is somewhat interesting.  That's coming from
nsCSSFrameConstructor::AppendFrames() and is only needed to determine whether we
have an :after pseudo!  Waterson has a patch somewhere that alleviates that
problem significantly, if I recall correctly.

The BuildFloaterList() thing has also come up in the past as something we do a
lot for little reason....

Why is the UndisplayedMap involved?  There is no "display:none" content on this
page...

Comment 12

[Boris Zbarsky [:bzbarsky]]

Actually, forget that.  There _is_ display:none content, of course.  The
<script> tags...

Comment 13

[Boris Zbarsky [:bzbarsky]]

There is the secondary issue (and that's part of the problem here) that
UndisplayedMap::AppendNodeFor is linear in the number of display:none children
of the parent of the content being added... if we can eliminate that uniqueness
check in opt builds, could we perhaps store a pointer to the tail of the list also?

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We could eliminate the uniqueness check by making undisplayed map entries hold
strong pointers to content, just like frames do, which is probably what I should
have done to solve that crash in the first place.

Other than that, this looks pretty much like the standard list of things that
are O(N) in the size of the existing content or frame structure for each
incremental reflow (making it O(N^2) overall).  Bug 86952 has some useful
pointers (see http://bugzilla.mozilla.org/showdependencytree.cgi?id=86952 )
although a bunch of the relevant problems don't have bugs, only nasty comments
in the code.

Presumably the time in GetCombinedArea is from the loop in
nsBlockFrame::ComputeFinalSize.

Why are we painting so much?  Is something in the reflow process doing
invalidates that shouldn't be?  Or is that because of the details of this testcase?

Comment 15

[Markus Hübner]

Is painting/reflow still that bad?

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

can we close this bug? No testcase, no input for 6 years

Comment 17

[Carlos Alén Silva]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4) Gecko/20100503 Firefox/3.6.4

I was able to reproduce on Fx 3.6.4 using the test case in comment #8. Fx hung/froze for a few seconds, with 100% CPU usage. :(

Comment 18

[Boris Zbarsky [:bzbarsky]]

This is 2x faster than Chrome now.  Still pretty bad in overall terms, but the page is explicitly doing 1000 reflows, and we're loading it in about 10s, so 10ms per reflow.  That's not horrible, at least... Marking worskforme.