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

RESOLVED WORKSFORME

Status

()

P3
critical
RESOLVED WORKSFORME
19 years ago
11 years ago

People

(Reporter: greg, Unassigned)

Tracking

(Blocks: 1 bug, {dom0, perf, testcase})

Trunk
Future
dom0, perf, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

19 years ago
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

19 years ago
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.
Assignee: rogerl → vidur
Component: Javascript Engine → DOM Level 0
QA Contact: rginda → desale
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().
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: beta1
OS: Linux → All
Hardware: PC → All
Summary: Browser locks when rendering GIMP manual toc frame → Loading page is *really* slow (due to bad document.write() perf)
Whiteboard: [HAVE FIX]
Target Milestone: M14

Comment 3

19 years ago
Putting on PDT- radar for beta1.  Please put a patch in the bug so we can do
this after branch.
Whiteboard: [HAVE FIX] → [PDT-][HAVE FIX]
Created attachment 6359 [details] [diff] [review]
document.write() speedup patch.

Updated

19 years ago
Target Milestone: M14 → M15

Comment 5

19 years ago
Created attachment 6500 [details]
testcase: for loop with 1000 document.write's timed

Comment 6

19 years ago
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
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...
Target Milestone: M15 → M17

Updated

19 years ago
Keywords: perf, testcase
Whiteboard: [PDT-][HAVE FIX] → [PDT-][HAVE FIX][TESTCASE]

Comment 8

19 years ago
Isn't this a dup of bug 23187?

Comment 9

19 years ago
Adding nsbeta2 to keywords.
Keywords: nsbeta2
Whiteboard: [PDT-][HAVE FIX][TESTCASE] → [HAVE FIX][TESTCASE]

Comment 10

19 years ago
Putting on [nsbeta2-] radar.  
Whiteboard: [HAVE FIX][TESTCASE] → [nsbeta2-] [HAVE FIX][TESTCASE]

Comment 11

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

updating keywords & dependency
Blocks: 23187
Keywords: patch
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.
Target Milestone: M17 → Future

Comment 13

18 years ago
Nominating for nsbeta3 since it was in for nsbeta2 too.
Keywords: nsbeta3

Comment 14

18 years ago
Vidur says to give this to Jst.
Assignee: vidur → jst
Status: ASSIGNED → NEW
Depends on: 43902
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.
Whiteboard: [nsbeta2-] [HAVE FIX][TESTCASE] → [nsbeta2-][nsbeta3-][HAVE FIX][TESTCASE]

Updated

18 years ago
Status: NEW → ASSIGNED

Comment 16

18 years ago
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.

Depends on: 61842
Removing old keywords and leaving as Futured for now.
Keywords: beta1, nsbeta2, nsbeta3, patch
Whiteboard: [nsbeta2-][nsbeta3-][HAVE FIX][TESTCASE]
Keywords: dom0

Comment 18

17 years ago
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).
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

17 years ago
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

17 years ago
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.
Keywords: mozilla1.0

Comment 22

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

Comment 23

17 years ago
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

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

Comment 25

17 years ago
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.

Updated

17 years ago
Blocks: 117611

Updated

17 years ago
No longer blocks: 117611

Updated

17 years ago
Blocks: 118933

Updated

17 years ago
Keywords: mozilla1.0+

Updated

17 years ago
Keywords: mozilla1.0
I'll run a jprof of this when I get a chance (monday or earlier)
Created attachment 72501 [details]
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

17 years ago
Created attachment 74529 [details]
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

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

Comment 30

17 years ago
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.

Updated

17 years ago
Attachment #74529 - Attachment mime type: application/octet-stream → text/plain

Updated

17 years ago
Keywords: mozilla1.0+ → mozilla1.0-

Comment 31

16 years ago
Ran the testcase in comment 5: 

IE 6 : 250
Opera 7 b2: 931
Moz 1.3a: 1103
Keywords: mozilla1.3
Mass-reassigning bugs to dom_bugs@netscape.com
Assignee: jst → dom_bugs
Status: ASSIGNED → NEW
The top culprit in a current jprof is the last-frame lookup when checking for
:after generated content.
Depends on: 145425

Comment 34

13 years ago
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

12 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 36

12 years ago
No specific bug / patch referenced as the fix.

-> WORKSFORME
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

12 years ago
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.