29805 - Loading page is *really* slow (due to bad document.write() perf)

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect, P3)

Description

[greg]

From Bugzilla Helper:
User-Agent: Mozilla/4.71 [en] (X11; I; Linux 2.2.13-7mdk i686)
BuildID:    2000022808

The browser locks up (CPU=100%) while rendering the Javascript-heavy toc frame
for the GUM. (At least, I'm assuming it's the Javascript) No errors come across
the console when this happens. It happens in both Linux and in Windows
(build#2000030108 for the latter).
http://manual.gimp.org/manual/GUM/GUM.html

Reproducible: Always
Steps to Reproduce:
1. Go to URL.
2.
3.

Actual Results:  Browser locked.

Expected Results:  Shown the page.

Comment 1

[rogerl (gone)]

With build from 3/7 i don't get a lock, but it does take a very long time to 
finish loading, and when it does the TOC tree on the left is fully opened.

Comment 2

[Johnny Stenback (:jst)]

The reason loading the above URL takes a really long time is that the page
contains a TOC of the manual and that TOC is created by doing thousands of
document.write() calls, from discussing with Vidur it turns out that every
document.write() call made from javascript causes the document to iterate
through all content nodes in the document searching for an item named "write"
(in this case), this is *really* bad for performance, and all (most)
document.write() calls add more content to the document and thus the following
document.write() call takes even longer to execute. Me and Vidur came up with a
temporary fix that makes the above URL load in about 12 seconds in stead of 2
minutes.

The fix is safe (almost a oneliner), all it does is check if the named item
that is searched for in the document is named "write", "writeln", "open" or
"close", if so it just returns without traversing all content nodes in the
document only to come to the same conclusion (that there is no such item).

This fix will considerably speed up almost every webpage that uses
document.write().

Comment 3

[leger]

Putting on PDT- radar for beta1.  Please put a patch in the bug so we can do
this after branch.

Comment 4

[Johnny Stenback (:jst)]

Attachment: document.write() speedup patch.

Comment 5

[Peter 'Rufus' Nelson]

Attachment: testcase: for loop with 1000 document.write's timed

Comment 6

[Peter 'Rufus' Nelson]

this is my results from the testcase (w/o patch):
Net 4.7   ~12000
Moz m14   3020
IE 5.0    550

as you can see we're significantly better than 4.7, but no where near IE

Comment 7

[Johnny Stenback (:jst)]

The patch (or a similar patch) is checked in, we're doing much better now but I
think there's still room for optimizing this, I won't close this bug yet, I'll
move it to M17 for now...

Comment 8

[Peter "jag" Annema]

Isn't this a dup of bug 23187?

Comment 9

[Prashant Desale]

Adding nsbeta2 to keywords.

Comment 10

[leger]

Putting on [nsbeta2-] radar.  

Comment 11

[fig\tree]

bug 23187 is a tracking/perf bug where as this bug covers a specific issue
slowing down document.write()

updating keywords & dependency

Comment 12

[Johnny Stenback (:jst)]

This bug has been marked "future" because the original netscape engineer
working on this is over-burdened. If you feel this is an error, that you or
another known resource will be working on this bug,or if it blocks your work
in some way -- please attach your concern to the bug for reconsideration.

Comment 13

[Prashant Desale]

Nominating for nsbeta3 since it was in for nsbeta2 too.

Comment 14

[Warren Harris]

Vidur says to give this to Jst.

Comment 15

[ekrock's old account (dead)]

Marking as [nsbeta3-] a number of bugs that were already marked Future (but not 
[nsbeta3-]) because the Netscape engineer the bug is assigned to is 
overburdened. If you disagree with this decision, please provide information 
about customer and user impact, but please do not clear the [nsbeta3-] unless 
you are reassigning the bug to yourself and committing to a fix within the 
nsbeta3 timeframe.

Comment 16

[bht237]

The speed of any real world document.write() currently also depends on bug
61842.
Bug 61842 deals with a shocking slowdown due to parsing of script that contains
HTML.
In other words: Even if document.write() is fixed, performance will be not as
good as expected.
However, the dependency exists only wrt the test result and possibly not coding
i.e. when Bug 61842 is fixed then the page this bug is concered with will render
faster anyway.

Comment 17

[Johnny Stenback (:jst)]

Removing old keywords and leaving as Futured for now.

Comment 18

[Daniel Jensen]

The fixing of the dependencies has not fixed the real-world problem as the last
comment attests- the GUM still is terribly slow, locking my browser for roughly
4 minutes when simply taking the material out of the cache! (IE 5.5 does the
same in about 30 seconds, I haven't been able to reproduce a similar situation
with NS 4.77.) On my PII 400 Win32: testcase computes 2630 on Moz .9.2, 1380 on
NS 4.77, and 330 on IE 5.5, with fairly large standard deviations (about 20) on
Moz and NS but practically none on IE. The correlation between the GUM time
difference and the testcase time difference is extremely close- roughly 8x vs
rather exactly 7.97x the IE time, so I think we can infer that the testcase
accurately represents the problem without giving consideration to the (resolved)
 dependencies. This ought to be fixed for 1.0 (not that I have any authority to
set that milestone). In my opinion, checking the patch in ASAP before the real
push to 1.0 begins would be a Good Thing (TM).

Comment 19

[Johnny Stenback (:jst)]

The patch in this bug was checked in ages ago, and it did improve performace by
an order of magnitude, the code relevant to this patch is long gone by now.
Identifying and fixing the problems that remain more research and probably
non-trivial changes to mozilla, I wouldn't expect much of that happening for
mozilla1.0.

Comment 20

[Peter 'Rufus' Nelson]

Some quick tests of mine (avg):
Moz (2001070404): 1300
Net 4.7: 500
IE 5.5: 130

So yeah, this bug has come back with a vengance...

Comment 21

[James Green]

I just hit this site and had it not been that I'm used to Mozilla hanging for up
to 1 minute on slow JS/DOM ops, I would as a user assume that the page crashes
the browser.

Comment 22

[Asko Tontti]

*** Bug 90882 has been marked as a duplicate of this bug. ***

Comment 23

[Daniel Jensen]

The fact that Moz M14 was beating NS 4.7 *without* the patch while Moz .9.2 and
.9.3 lose to NS by taking almost all of twice the time NS 4.7 takes to do the
same   indicates to me a fairly major regression. I still think this is a 1.0
issue, but obviously the team has a lot on their platter and jst isn't very
happy to take the issue, so it ought to be given some sort of marking for later
so people remember it's a fairly major problem. You could put it as 1.0.1 or 1.1
tagged, that might do the job.

Comment 24

[Phil Schwartau]

*** Bug 72107 has been marked as a duplicate of this bug. ***

Comment 25

[Daniel Jensen]

Mozilla .9.8 gives 2080 on my system, considerably better than earlier versions
but not by the order of magnitude which it ought to progress by. NS 4.79 gives
1150, also attesting an improvement. IE 6 appears to be a good deal slower than
IE 5.5 on this test- 380 instead of 130. I don't know whether or not this is due
to a GUM site redesign, but the GUM is now usable- 40 second delay in rendering
the javascript compared to 4 minutes the first time I checked. However, keyword
editing would probably be a good idea (it now looks like this perhaps isn't a
significant DOM 0 issue nor a Mozilla 1.0 bug). If someone can verify my results
and find whether the speed on the page is due to the newer browser or a newer
website, please respond.

Comment 26

[Randell Jesup [:jesup] (needinfo me)]

I'll run a jprof of this when I get a chance (monday or earlier)

Comment 27

[Randell Jesup [:jesup] (needinfo me)]

Attachment: jprof of testcase

With trunk from 2/28/02 roughly, Linux 7.2, dual-450 P3.  Time is 1150ms.
Nothing horribly obvious to rough inspection: 100 hits, 27% in
nsHTMLDocument::Write, 22% in popping the Alert, 26% in
XPC_WN_Helper_NewResolve()

The rest is scattered around.

Comment 28

[Daniel Jensen]

Attachment: Flat profile from a PII400 machine

I'm not very good with Venkman, but it seems to me data from something other
than a dual-processor system is required. Here goes.

Comment 29

[Daniel Jensen]

-that is not application/octet-stream, sorry I forgot to set the mime type-

Comment 30

[Daniel Jensen]

20 days spent and (hopefully) less than a month before the 1.0 keyword and
counting... If we're sure about this being a bug which is distributed across a
bunch of different operators, it's probably a good idea to remove the 1.0+ keyword.

Comment 31

[José Jeria]

Ran the testcase in comment 5: 

IE 6 : 250
Opera 7 b2: 931
Moz 1.3a: 1103

Comment 32

[Johnny Stenback (:jst)]

Mass-reassigning bugs to [e-mail]

Comment 33

[Boris Zbarsky [:bzbarsky]]

The top culprit in a current jprof is the last-frame lookup when checking for
:after generated content.

Comment 34

[nick]

Just for fun, I ran the test case in comment #5 more than five years after this
bug was posted on a Athlon64 3200+ running Windows XP. The results:

IE 6.0: 65 ms
Firefox 1.0.6: 140 ms
Deerpark nightly: 95 ms

The improvements in Deerpark suggest that this performance issue is eventually
fixed.

Comment 35

[Peter 'Rufus' Nelson]

Doing a bit of reminiscing I came back to this bug that I testcase'd 6 years ago.  Testing on my WinXP machine with a testcase that does 10,000 writes I get:
Firefox 1.5.0.4: 470
IE 6.0: 375

So we're a bit slower, but it's no longer the problem that it used to be.  Marking as fixed.

Comment 36

[Jason Bassford]

No specific bug / patch referenced as the fix.

-> WORKSFORME