12297 - [dogfood]{perf} Assertion: "bad line child count" (line->mChildCount < 10000) at file ../../../../../layout/html/base/src/nsBlockFrame.cpp, line 6034

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

When viewing this page, which constitutes basically one enormous table, I get
approximately a billion of those assertions and then:

Block(pre)(5)@0x88cad18: WARNING: desired:9240,505682
nsBlockReflowContext: Block(pre)(5)@0x88cad18 metrics=9240,505682!
Block(body)(1)@0x86d6fa8: WARNING: desired:9240,508249
nsBlockReflowContext: Block(body)(1)@0x86d6fa8 metrics=9240,508249!
Area(html)(-1)@0x8681938: WARNING: desired:9480,508569
Block(pre)(5)@0x88cad18: WARNING: desired:9015,505682
nsBlockReflowContext: Block(pre)(5)@0x88cad18 metrics=9015,505682!
Block(body)(1)@0x86d6fa8: WARNING: desired:9015,508249
nsBlockReflowContext: Block(body)(1)@0x86d6fa8 metrics=9015,508249!
Area(html)(-1)@0x8681938: WARNING: desired:9255,508569
Block(body)(1)@0x86d6fa8: WARNING: desired:9015,508713
nsBlockReflowContext: Block(body)(1)@0x86d6fa8 metrics=9015,508713!

etc.

I get some mangled display below the line numbered 2440 or so when I scroll
down, but that's probably something other than layout's fault because I also get
chrome content rendered in there when cause a repaint.

Resizing is a substantial time commitment, too. =)

Comment 1

[troy]

About a billion, huh? Wow, I guess Kipp will have to handle this, I can't count
that high. :-)

Comment 2

[kipp]

Need to add in code to handle outrageous lines too...

Comment 3

[kipp]

I checked in a partial workaround for this bug (I raised the assertion limit to
50,000 from 10,000); however, the bug is still valid because there is a limit
and we don't handle it well.

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Egads, M17?  I can't use LXR with the current setup, which is a big part of
``dogfood'' for many of our engineers.  Can I or another porkjockey lend a hand
fixing this, or taking some other bug off your plate to make room?

Comment 5

[kipp]

Umm, how about you run into the new assertion first before you complain about
it... :-) Thanks.

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Um, well.  If you go to
http://lxr.mozilla.org/mozilla/source/widget/src/windows/nsWindow.cpp in
apprunner, you no longer get the assertion, but you pretty much lose the
browser.  After a little while the progress bar stops and you can no longer
interact with the UI (doesn't redraw when you obscure and expose, etc.).

Popping into it and sampling with gdb gives us traces like:
#0  0x4012db2f in nsVoidArray::operator[] (this=0xbfffdc04, aIndex=145641656)
    at ../../../xpcom/ds/nsVoidArray.h:43
#1  0x40e0ca78 in nsTextFrame::FindTextRuns (this=0x8ae50b8,
    aLineLayout=@0xbfffdc04)
    at ../../../../../layout/html/base/src/nsTextFrame.cpp:1592
#2  0x40dd4252 in nsBlockFrame::ComputeTextRuns (this=0x89cd760,
    aPresContext=0x858b248)

and we spend a lot of time in this section of nsLineLayout::AddText.

2081    #ifdef DEBUG_ADD_TEXT
2082      PRInt32 n = mNewTextRun->mArray.Count();
2083      for (PRInt32 i = 0; i < n - 1; i++) {
2084        NS_ASSERTION(mNewTextRun->mArray[i] != (void*)aTextFrame, "yikes");
2085      }
2086    #endif
2087      return NS_OK;/* XXX */
2088    }
(gdb) print n
$1 = 10039
(gdb) print i
$2 = 6014

[another time]
(gdb) print n
$3 = 12953
(gdb) print i
$4 = 5076

[another time]
#0  0x40dd4b27 in nsBlockFrame::VerifyLines (this=0x89cd760, aFinalCheckOK=1)
    at ../../../../../layout/html/base/src/nsBlockFrame.cpp:6257
6257        frame->GetNextSibling(&frame);
(gdb)
6255      while (nsnull != frame) {
6256        frameCount++;
6257        frame->GetNextSibling(&frame);
6258      }
(gdb) print frameCount
$6 = 9709
(gdb) print count
$7 = 15011

Some of these look like debug-only stuff, which is OK.  Is the frame-counting
thing a required runtime check, or just sanity for count == frameCount?  (Maybe
we should have a pref or something for VerifyLines?)

I'll try hacking out some of the verification stuff to see if it's really to
blame for the performance issues, which really do render it unusable for LXR
work.

You did indeed fix the assertion; should I file another bug for this stuff?  I
don't mind.

Comment 7

[kipp]

Yeah - please do file another bug for the performance issue so that I pay
attention to it...thanks!

Comment 8

[kipp]

Any status on this? Is LXR usable w/o the VerifyLines and TextRun verification
logic enabled?

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Sorry for the delay -- I can't build just layout without debug, because I get
nsCOMPtr linkage errors, so I'll have to clobber and rebuild the whole thing.
Once I get finished tracking down this crash I'm looking at, I'll do just that.

Comment 10

[kipp]

Moving to M12 for performance analysis. It's hideously slow to load the test
page...

Comment 11

[leger]

Putting on PDT+ radar.

Comment 12

[kipp]

Status update: I've landed code to reduce the memory consumption of the frame
model a few precentage points. In addition, I've upped the number of frames per
line from 65535 to (2^24)-1. This should eliminate any future issues with the
assertions (which are now gone...). The speed issue remains.

Comment 13

[kipp]

Since the speed issue is covered by 7455 and the assertion issue has been fixed,
I'm closing this bug.

Comment 14

[Chris Petersen]

Marking as verified fixed in the Oct 22 build.