RESOLVED FIXED in mozilla1.7beta

Status

()

P1
critical
RESOLVED FIXED
16 years ago
13 years ago

People

(Reporter: baldauf--2015--bugzilla.mozilla.org, Assigned: brendan)

Tracking

({js1.5, perf})

Trunk
mozilla1.7beta
js1.5, perf
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments)

(Reporter)

Description

16 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030718
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030718

When accessing http://www.joki-foto.de/fotoshop/fehler.htm, mozilla seems to
loop endlessly. No user interaction will be possible anymore. I suspect a loop
within the JavaScript code of that page. But even if there is such a loop, the
user should be able to continue browsing instead of killing mozilla.


Reproducible: Always

Steps to Reproduce:
1. Access http://www.joki-foto.de/fotoshop/fehler.htm
2. Try to do anything with mozilla (i.e. open a new browser windows)
Actual Results:  
Mozilla hangs. It eats all available CPU time.

Expected Results:  
Mozilla should not hang. Mozilla should still do user interaction. Mozilla
should  enable the "Stop" button so that some looping JavaScript code can be
interrupted if the user desires that.

Comment 1

16 years ago
Works for me 2003090505 Linux after stalling for 45 seconds.

I suspect this is due to the 1.66 megabyte JS file it links to at
http://www.joki-foto.de/fotoshop/search.js

JS console gives no errors.

Comment 2

16 years ago
WFM Gecko/20030902 Firebird/0.6.1+. Does not hang, it just takes long time to
load the page. 

Updated

16 years ago
Severity: normal → critical
Keywords: hang
Not likely to be a JS engine bug per se... over to General until we have a
minimal testcase that shows the problem.

If this is just a perf issue, perhaps a profile of the pageload is in order?
Keywords: qawanted
.
Assignee: rogerl → general
Component: JavaScript Engine → Browser-General
QA Contact: pschwartau → general

Comment 5

15 years ago
I'm working up a reduced testcase for this. As Jose noted, the page does load
eventually, but it takes well over a minute even on a reasonably new machine.

In a nutshell, the perf problem looks like it's caused by the page creating a
massive array of arrays in Javascript. The source code that creates the arrays
is roughly 1.7 megabytes of text. The first few lines look like this:

var articles = new Array(
new Array('1 Agfa Pan 100 120','150656','','','2.87','article_g000_000.html',''),
new Array('1 Agfa Pan 100 135/24','577189','','','2.81','article_g000_001.html',''),

and so on for hundreds of lines. While this is not great coding practice, IE6
does manage to render it in about 10 seconds as opposed to 2 minutes so it seems
like something could be improved. If it will help I can attach the full 1.7
megabyte testcase, or a smaller version that still gets the point across.

This may be a dupe of or related to bug 171262.
Does it just create the array, or does it create DOM nodes from it?  I would
assume the latter...

If you have a functioning testcase that you can zip up (due to the size this may
be needed) and attach here that shows the pageload issue, please go for it.

Comment 7

15 years ago
Created attachment 142747 [details]
Testcase

Here is the zipped testcase I created.
Well, how interesting.  This _is_ a JS engine issue after all.  Though I do
wonder why the fix for bug 13350 is not kicking in.

The profile shows:

Total hit count: 10809
Count %Total  Function Name
10666   98.7     GetChar
 27   0.2     FindCharInReadable
  7   0.1     nsTextFragment::SetBidiFlag
  6   0.1     memmove

Everything else is below the 0.1% level.

Of the 10666 hits in GetChar, we have (more or less; the sum will be greater
than 10666 because a few of those hits are actually under js_AtomizeChars, not
GetChar):

   572 coming from js_GetToken
   174 coming from js_PeekToken
   131 coming from UnaryExpr
  1189 coming from js_MatchToken
  8751 coming from UnaryExpr

It looks like the only callstack that ends up in UnaryExpr is:

UnaryExpr
MulExpr
AddExpr
ShiftExpr
RelExpr
EqExpr
BitAndExpr
BitXorExpr
BitOrExpr
AndExpr
OrExpr
CondExpr
AssignExpr

10704 of the total 10809 hits are under AssignExpr.

Moving on up, it looks like most of that stuff is under

AssignExpr
Variables
Statements
js_CompileTokenStream  (10707 of the total hits).

That probably explains why the DOM can't interrupt this mess -- this is an
atomic compilation operation....
Assignee: general → general
Status: UNCONFIRMED → NEW
Component: Browser-General → JavaScript Engine
Ever confirmed: true
Keywords: hang, qawanted → perf
OS: Windows XP → All
QA Contact: general → pschwartau
Hardware: PC → All
The issue seems to be that the script is all a single line.  After running:

  perl -pi -e 's/new Array/\nnew Array/g;' testcase.htm

the file loads in Mozilla in about five seconds (still freeze up for those five
seconds, though).
(Assignee)

Comment 10

15 years ago
Big fun -- thanks for the profile, bz.

(No one should put everything on one line!  That's just wrong ;-)

/be
Assignee: general → brendan
(Assignee)

Comment 11

15 years ago
Created attachment 142781 [details] [diff] [review]
proposed fix (apply this)

Reviewers: diff -w version coming right up.

/be
(Assignee)

Comment 12

15 years ago
Created attachment 142783 [details] [diff] [review]
diff -w version of last patch (review this)
(Assignee)

Updated

15 years ago
Attachment #142783 - Attachment description: diff -w version of last patch → diff -w version of last patch (review this)
Attachment #142783 - Flags: review?(shaver)
(Assignee)

Updated

15 years ago
Status: NEW → ASSIGNED
Keywords: js1.5
Priority: -- → P1
Target Milestone: --- → mozilla1.7beta
Comment on attachment 142783 [details] [diff] [review]
diff -w version of last patch (review this)

I like pathology. r=shaver
Attachment #142783 - Flags: review?(shaver) → review+
(Assignee)

Comment 14

15 years ago
Fixed.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
OK, with that patch 6 pageloads give me:

Total hit count: 1087

Count %Total  Function Name
171   15.7     FindCharInReadable
 78   7.2     GetChar
 45   4.1     nsTextFragment::SetBidiFlag
 41   3.8     memcpy
 31   2.9     FindInReadable
 30   2.8     memmove
 28   2.6     js_GetToken
 21   1.9     js_MatchToken
 20   1.8     js_SearchScope

So GetChar is still up there, but it's much better now... (like nearly 4 orders
of magnitude better).

I filed bug 236272 on looking into making the the code that's calling
FindCharInReadable (CTextToken::ConsumeUntil) a litte faster.

Updated

14 years ago
Flags: testcase?

Updated

13 years ago
Flags: testcase? → testcase-
You need to log in before you can comment on or make changes to this bug.