Closed Bug 19051 Opened 20 years ago Closed 20 years ago

{ib} extremely slow layout of page with many inline tags (<PRE> involved?)

Categories

(Core :: Layout, defect, P1, major)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: barath, Assigned: waterson)

References

()

Details

(Keywords: perf, Whiteboard: [HAVE FIX])

Attachments

(5 files)

I'm using Milestone 11 on a standard Redhat Linux 6.1 box, and viewing an image
that has been turned into a graphic such as the url provided, mozilla goes
extremely slow and becomes unresponsive for a long time.  This is in contrast to
netscape communicator or IE, which render and scroll with such an image
(text-image) rather quickly.

Thanks.
the page is not there anymore
Reassigning all of leger's unscreened Browser-General bugs to nobody@mozilla.org
for pre-screening and triage.
Whiteboard: (12/28) invalid URL; need an example to proceed further
barath@uclink4.berkeley.edu : can you check the URL for this bug report. I
don't understand the problem from the description, and the URL provided is
404 Not Found.
Assignee: nobody → pierre
Component: Browser-General → Style System
Whiteboard: (12/28) invalid URL; need an example to proceed further
From barath@uclink4.berkeley.edu, by email to 3jrgm@qlink.queensu.ca:

  "Try using the program png2html to create a html based image and then view
  it in mozilla."

So, as an example page already on the web, we can just use the png2html home
page : http://www.engr.mun.ca/~holden/png2html.html [Barath : is this a good
example of what you are seeing? (Add comments to this bug report).]

Using WinNT 1999122208, this takes roughly twice the time to render, as compared
to Nav4.7 on WinNT. However, I note that this bug report is specifically for
Linux, so the display may be even slower on Linux (based on the original
description [barath: can you quantify how much slower vs. Nav4.7 on Linux]).

Setting to 'Style System'/pierre for a further look (but maybe this is a
painting issue). (It's a fairly degenerate example, of course, but may point
out some possible areas to tune up.)
Assignee: pierre → troy
Component: Style System → Layout
OS: Linux → All
Hardware: PC → All
Summary: extremely slow rendering of text-based images → [perf]extremely slow rendering of text-based images
It's slow on all 3 platforms. It is especially slow when:

- loading the page

- scrolling

- selecting several lines of text at once



Running with MOZ_PERF shows an unusally long reflow time. Reassigned to Troy.
Assignee: troy → kipp
On my machine running NT and with the latest build it loads in a very reasonable
time.

However, it could be faster and so since it's all done with preformatted text
this is the known issue that the block code needs to be optimized for
preformatted text
Target Milestone: M15
mass-moving bugs to M15
Keywords: perf
Summary: [perf]extremely slow rendering of text-based images → extremely slow rendering of text-based images
this is actually pretty darn fast now.  acceptable for beta, moving to M16, 
lowering priority.  won't close out yet, this is still an interesting test case 
to profile.  If anybody wants to run a quantify profile of this, it would be 
VERy helpful!
Keywords: helpwanted
Priority: P3 → P4
Target Milestone: M15 → M16
*** Bug 24766 has been marked as a duplicate of this bug. ***
Upping the severity of this, as it makes mozilla completely unusable for viewing
large files in lxr such as:

http://lxr.mozilla.org/seamonkey/source/include/allxpstr.h#7570
Severity: normal → major
Summary: extremely slow rendering of text-based images → extremely slow rendering of <PRE> blocks
kipp is very unlikely to fix these, since he's not working ont he project any 
longer.  so I'll take a look.
Assignee: kipp → buster
Now that we are post-beta, is there anyone actively working with this? It really
hurts LXR, and I have a gut feeling (but no data) that it's probably hard on
xmlterm too.
well, post-beta branch anyway. I submitted too quickly :-)
I doubt anyone will work on this before M17
performance issues are high priority now.  Will try to look at this for M16.
Status: NEW → ASSIGNED
Priority: P4 → P1
changed the summary.  It's a layout issue, not rendering. And <PRE> may or may 
not be involved at all.  It may be the sheer number of inline tags, in this 
case, <FONT>.  Trying to get quantify to play nice...
Summary: extremely slow rendering of <PRE> blocks → extremely slow layout of page with many inline tags (<PRE> involved?)
Target Milestone: M16 → M17
one thing I noticed in quantify is the {ib} code to find "special" frames is 
killing us on large documents with long text runs.  Nisheeth is scheduled to fix 
this soon.  Nisheeth, when this part of the problem is fixed, please re-assign 
this back to me so I can find the next problem.

basically, the {ib} 'special' frame code is killing us, even though no inline 
element contains a block element.  ugg!
Assignee: buster → nisheeth
Status: ASSIGNED → NEW
Summary: extremely slow layout of page with many inline tags (<PRE> involved?) → {ib} extremely slow layout of page with many inline tags (<PRE> involved?)
Blocks: 40246
Accepting bug...
Status: NEW → ASSIGNED
taking
Assignee: nisheeth → waterson
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Discovered a couple of things.

1. Long text runs with inlines get hosed because nsTextRun stores the run in a 
void array. Each span ends up as an entry here.

2. <pre>-formatted text with markup in it gets royally screwed here: generally 
the preformatted text is long, and for some reason we don't break the text run 
at the newline. It seems like we should probably do that. Any reason not to?

3. FindNextText() is called after measuring each new word: when the nsTextRun 
gets long, this becomes geometric and blows up.

You'll have to experiment with suggestion (2).  Off-hand, it sounds like it 
should be ok.
*spam* changing qa contact from nobody@mozilla.org to me (BlakeR1234@aol.com) 
on 121 open or resolved (but not verified) bugs.  sorry for the spam everybody, 
but most of these bugs would just remain dormant and not checked by QA 
otherwise.  I'm not sure how so many bugs have nobody as their QA contact, but 
I suspect this is the fault of some sort of bugzilla corruption that happened 
at some point (most of these bugs are in the 20000-26000 range, and I don't see 
where in the activity log that QA contact explicitly changed to 
nobody@mozilla.org)

Anyways, sorry again for spam.  If you really get annoyed, I'm usually 
available in #mozilla on IRC for torture.
QA Contact: nobody → BlakeR1234
So I think the second patch is a better approach. Namely, get rid of nsTextRun 
altogether, and make nsLineLayout::FindNextText() grovel over the frame tree to 
find the next text frame.

The only concern that I have with this patch is that I undid the lazy parent 
pointer fixup stuff that troy did to fix bug 5588. Specifically, when overflow 
frames are pulled to newly created line boxes, troy had it set up s.t. their 
parent pointers would continue to refer to their original frames until reflow 
occurred. This makes it difficult to walk the frame tree :-).

I experimented with the test cast in 5588. Specifically, I backed troy's 
changes out of an otherwise clean tree. I was not able to observe a significant 
difference in performance (using the ultra-scientific "one mississippi, two 
mississippi" method).
     Subject: Re: nsTextRun
        Date: Thu, 13 Jul 2000 08:28:47 -0700
        From: Troy Chevalier <troy@tellme.com>
Organization: Tellme
          To: Chris Waterson <waterson@netscape.com>

Chris Waterson wrote: 

  There is a fly in the ointment, though. I needed to undo the change that 
  you made to lazily set the overflow frames' parent pointers when 
  reflowing new lineboxes that get created on resize reflows. (I suppose I 
  could leave the change in, and add another eager detection case, but...) 

  I went back to the original test case (that big javadoc thing), and 
  honestly couldn't see a visible difference in layout speed after 
  removing the lazy parent pointer stuff. Was there another, better test 
  case I should look at?

That's an interesting issue. The main reason you don't see a layout change is 
because of this code in nsInlineFrame::Reflow(): 

  // When pushing and pulling frames we need to check for whether any 
  // views need to be reparented. 
  nsHTMLContainerFrame::ReparentFrameViewList(aPresContext, prevOverflowFrames, 
                                              prevInFlow, this); 

That's a new function that I added. Previously what happened is that in a loop 
we would call ReparentFrameView() once for each frame in the overflow list. That 
was the major performance problem, because each call did the exact same [very
expensive] check to see whether we needed to reparent the view or not 

Now that we make just the one call we only do that work once. Note that there is 
lots of layout code that is still using the old way, and it doesn't matter in 
galley mode but it's a performance problem in page mode 

There was a second reason performance was suffering, and that was because 
nsFrameList::InsertFrames() was walking the child list twice. The first time 
through it was calling SetParent() for each frame, and then it was calling 
LastChild() to get the last frame in the list. I changed the code that sets the 
parent frame to remember the last frame, and that made in O(n) instead
of O(2n) 

That may not seem like a big deal, but the scenario that was causing problems 
was something like this: 

<body> 
<span> 
everything in this span (100,000 elements) 
</span> 
</body> 

That's the worst case scenario, because we reflow the first line and only five 
word fit. So we push 99,995 elements to the next line, and for each one we 
called ReparentFrameView() and did the O(99,995 * 2) InsertFrames() operation. 
Then we fit five frames on the next line and pushed 99,990 frames to the next 
line, ... 

You get the idea. 

So skipping setting of the parent frame pointer if possible is still a good 
idea, because it saves us O(n). It's only an issue for large documents, but 
that's always been the problem area for us 

-Troy 
  
I tried the test case that troy described (1Mb of text in a span), and saw no 
measurable difference with and without my patch. I *did* see a small difference 
on the original test case from bug 5588:

  http://java.sun.com/products/jdk/1.3/docs/api/allclasses-frame.html

On my machine, 4.1s without the patch, 5.4s with the patch (that's a degradation  
of over 20%). I'm profiling now to see what I can find. That test case has a 
long run of formatted HTML inside a table cell.
In a subsequent email, troy pointed out that my test case was invalid because it 
only contained a single, long text frame. I tried it on a much larger test case 
(an LXR page contained inside of a <span> instead of a <pre>), and noticed 
marginal differences: 29s without the patch, 32s with the patch (10% 
degradation).

On LXR *with* a <pre> tag, the patch is a bigger win: from 35s without the patch 
do 25s with the patch (28% improvement).

At this point, my feeling is that removing the lazy parenting, and doing frame 
crawling is a net win:

1. (+) we avoid updating nsTextRun during incremental reflows
2. (+) we avoid storage overhead of the nsTextRun data (which saves us an 
nsVoidArray and its contents per block frame)
3. (-) we need to keep the parent pointers in sync eagerly to support frame 
walking during line wrapping.

I'll update my patch to leave the mSetParentPointer in mInlineFrame, and will 
experiment with this later to see if it's possible to do both lazy parent 
setting and frame crawling.
Keywords: helpwantednsbeta3
[HAVE FIX] for perf issue. PDT please approve.
Whiteboard: [HAVE FIX]
Target Milestone: M17 → M18
Blocks: 39133
Blocks: 26030
Add self to CC

In an attempt to say something useful in this spam, this bug:
http://bugzilla.mozilla.org/show_bug.cgi?id=2611
has some performance numbers on Mac for JDK 1.1 JavaDoc, which may be of 
interest.
fix checked in. bye bye, nsTextRun.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Can you let us know which nightlies have this in, so I can check whether the 
Javaoc loading issues have been solved?
M18 builds from 2000-07-28 and later have the fix.
Doing the same test on the JavaDoc file AllNames.html that I was talking about in 
bug 2611, the 28 Jul M18 build performs as follows:

Loading from network @ java.sun.com: 26.4 seconds
Loading from local disk:29.8 seconds

Conversley the M17 28 July build (which doesn't have this fix)

Loading from network: 60 secons
Loaing from disk: 90 seconds

So congrats - this fix is a huge improvement!

Still doesn't match Nav4 at 15 seconds from the local disk, but its pretty good.

I'm still curious as to how loading a file from disk can be *slower* than from 
the network. Should I file a bug on this?
cc'ing gagan: what do you think of the above comments? (re: network faster than 
file)
QA assigning to petersen to verify
QA Contact: blakeross → petersen
Marking verified in the 2001010203 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.