Closed
Bug 203699
Opened 22 years ago
Closed 15 years ago
fully unacceptable JavaScript performace with Germany's most famous household appliances company's homepage
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: franz.gans, Unassigned)
References
()
Details
(Keywords: perf, testcase)
Attachments
(7 files)
User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030312
Browse their homepage, please!
Reproducible: Always
Steps to Reproduce:
1. http://www.aeg-hausgeraete.de/
2. click and see how mozilla freezes
3.
Actual Results:
Mozilla needs 17 (seventeen) seconds to display the page.
While it is working, every Mozilla window freezes. Where
is the multi-threading?
Expected Results:
It should be effective and efficient.
|
||
Comment 2•22 years ago
|
||
no freeze with win2k build 20030426.. (or <1sec)
|
||
Comment 3•22 years ago
|
||
Confirming bug with Mozilla trunk binary 2003042808 on WinNT4.0.
OS: Linux ---> All.
The site takes about 16 seconds to load for me on WinNT. What seems to
take all the time is the document.write() of the menu frame at the left.
After 16 seconds elapse, suddenly the content of the menu appears.
I can't figure out what's going wrong, but it doesn't seem to be due to
pure JS Engine performance. The testcase I'll attach below will show this.
The site uses this frameset structure:
<frameset>
<frame name="menu" src="node10.asp">
<frameset>
<frame name="topFrame" src="node11.asp">
<frame name="main" src="node692.asp">
</frameset>
</frameset>
The "menu" frame has this body onload handler:
<body onLoad="parent.doMenu()">
The doMenu() function creates an abstract 133-element array as follows:
function doMenu()
{
var i=0;
arrMenuItems[i++] = new createMenuItem(692,9,"Home",1,"node692.asp","");
arrMenuItems[i++] = new createMenuItem(680,9,"Neuheiten",1,"node680.asp","");
arrMenuItems[i++] = new createMenuItem(371,9,"Produkte",1,"node371.asp","");
etc.
etc.
}
where the function createMenuItem() is simply:
function createMenuItem(id,pid,name,level,URL,target)
{
this.id = id;
this.pid = pid;
this.name = name;
this.level = level;
this.URL = URL;
this.target = target;
}
Meanwhile, the "main" frame has this body onload handler:
<body onLoad="fixMenu(692);" >
The 692 simply serves to highlight the first menu item (see above array).
This function, located in the frame's own script, calls up to the parent:
function fixMenu(ID)
{
if(parent.fixMenu)
{
if(ID)
parent.fixMenu(ID)
}
}
That is, the frame calls the fixMenu() function of the parent page.
In the testcase, I will only have this parent version of the function.
It document.writes() the menu into the "menu" frame by using the
arrMenuItems[] array above, and this sequence of function calls:
fixMenu()
---> prepareMenu()
---> buildMenu()
It's this last function, buildMenu(), that is causing the trouble.
It gets called 133 times, one for each element of arrMenuItems[].
Furthermore, it calls itself recursively, to recursion depth = 4.
I did a performance profile of the site using Venkman, Mozilla's
JavaScript debugger. I also did one for the reduced testcase I'll
attach below. The two profiles show a big difference in the time
the buildMenu() function takes in each case -
|
|
||
Comment 4•22 years ago
|
||
The reduced testcase will have this frameset structure:
<frameset>
<frame name="menu" src="about:blank" onload="doMenu();fixMenu(692);">
<frameset>
<frame name="topFrame" src="about:blank">
<frame name="main" src="about:blank">
</frameset>
</frameset>
That is, all the frames have src="about:blank", and the calls to
doMenu() and fixMenu() are made directly from the onload handler
of the "menu" frame. I only keep the other frames there to retain
the geometry of the page layout -
|
|
||
Comment 5•22 years ago
|
||
|
|
||
Comment 6•22 years ago
|
||
|
|
||
Comment 7•22 years ago
|
||
|
|
||
Comment 8•22 years ago
|
||
When you look at the Venkman profiles, don't forget the
call sequence above:
fixMenu()
---> prepareMenu()
---> buildMenu()
Therefore large execution times for the first two functions are
simply due to the large execution time for buildMenu(). This is
the biggest difference between the two profiles:
------------------------- At the site -------------------------
Function Name: buildMenu (Lines 115 - 163)
Total Calls: 133 (max recurse 4)
Total Time: 15703.13 (min/max/avg 15703.13/15703.13/118.07)
----------------------- In the testcase -----------------------
Function Name: buildMenu (Lines 242 - 290)
Total Calls: 133 (max recurse 4)
Total Time: 531.25 (min/max/avg 531.25/531.25/3.99)
You can see that in each case, the function is called 133 times
with a recursion depth of 4. The total time this function takes at
the website is 15,703 ms ~ 15.7 seconds, with an average of 118 ms
per call.
By contrast, the total time it takes in the testcase is ~ 531 ms,
with an average of 4 ms per call.
So for some reason, this function is taking approx. 30 times longer
when called on the website than it does when called in the testcase.
I do not know why that is, other than the change in frameset structure
that I made, or possible security domain checks that may be occurring
when Mozilla is loading the website.
At any rate, the pure JavaScript involved seems to be the same in each
case. One has to be careful here, and look at every logical step that
buildMenu() makes. For example, I verified |arrParents.length| === 1
at both the website and the testcase. Therefore in each case, we fall
into the |(arrParents.length > 0)| branch of the buildMenu() function.
One can verify this by loading each page and entering:
javascript: alert(arrParents.length);
Reassigning to DOM Level 0 for further analysis; cc'ing Mitch -
Assignee: rogerl → dom_bugs
Component: JavaScript Engine → DOM Level 0
QA Contact: pschwartau → ashishbhatt
|
|
||
Comment 9•22 years ago
|
||
To be precise, it is the prepareMenu() function that does the
document.write() into the menu frame:
menu.document.writeln(strStart + strHTML + strEnd);
The |strStart|, |strEnd| variables are defined inside the
prepareMenu() function, but without the |var| keyword, and
are therefore global variables.
The |strHTML| variable is declared at the top of the parent
script, so is also global. It is this string variable that
is set by the buildMenu() function, called from prepareMenu().
Summary: fully unacceptable java script performace with Germany's most famous household appliances company's homepage → fully unacceptable JavaScript performace with Germany's most famous household appliances company's homepage
|
|
||
Comment 10•22 years ago
|
||
Other points to note:
1. There are .gif images that load to the left of the menu items.
In the testcase, I gave them absolute URLs. At the website,
they have relative URLs.
2. The "main" frame includes a Shockwave Flash plug-in. The URL
for this frame is http://www.aeg-hausgeraete.de/node692.asp
|
|
||
Comment 11•22 years ago
|
||
|
|
||
Comment 12•22 years ago
|
||
I keep worrying that somehow not all codepaths in the buildMenu()
function are being exercised in the reduced testcase as they would
be at the website.
To investigate this, I used a more granular form of Venkman logging.
I set a breakpoint at the following line inside buildMenu():
if(arrParents[i] == arrMenuItems[j].pid)
This is at line 122 at the website, and at line 249 in the testcase.
Once this breakpoint is hit, I right-click this line in Venkman and
set these breakpoint properties:
Enable breakpoint: OFF
When triggered, execute: return __count__++;
Continue regardless of result: ON
Log result ON
I then double-click on "Apply" and hit "F5" to continue. This
takes time, as the line should be hit 133*133 = 17,689 times!
I found that it was hit exactly this number of times, whether
at the website or the testcase. So at least this part of the
codepath is exercised the same in each. There are other lines
to check as well; if I have time I will do that -
|
|
||
Comment 13•22 years ago
|
||
That's some pretty fancy breakpointing :) I think I'll add this to the FAQ as a
way to log specific functions. I'd just like to point out two things:
One, __count__ is a paremeter of the breakpoint script, so it goes away when the
function returns. Venkman maintains the breakpoint count elsewhere, and just
passes the correct value to the breakpoint script. The upshot of this is that
|return __count__++;| should just be |return __count__;|.
The second thing is that the "Enable Breakpoint" option is actually supposed to
disable the breakpoint altogether. "Continue regardless of result" is all you
should have needed to check to keep venkman from actually *stopping* for the
breakpoint. The fact that the breakpoint still happened means that there is a
bug in Venkman. I'll look into it.
|
|
||
Comment 14•22 years ago
|
||
Ah, the "Enable Breakpoint" problem was also the reason you had to double-click
the Apply button, instead of just single click it. Both problems are fixed in
0.9.67, at http://www.hacksrus.com/~ginda/venkman/.
|
|
||
Comment 15•22 years ago
|
||
This is caused by bug 169559. Try this:
1. Save some files locally. You need http://www.aeg-hausgeraete.de/ ,
http://www.aeg-hausgeraete.de/node10.asp and
http://www.aeg-hausgeraete.de/node692.asp to pollute the global namespace
sufficiently (I think that's why the testcase is not as slow). The 2 other
files need access to the main page, so they must be local too.
2. Put a <base href="http://www.aeg-hausgeraete.de/"> in every file and adjust
the frameset in the main page to point to your local files.
3. Load the page to make sure it's still slow.
4. Add a few missing |var|s in the script of the main page (at least for
|arrMenuItems| and |arrParents|; there are more, but they are less important)
and wrap the script within this code:
(function(){
// original script goes here
self.doMenu= doMenu;
self.fixMenu= fixMenu;
})();
5. Load the page again. It should be quite fast now.
6. Just to be sure, remove the |var|s added in step 4. The page is slow again.
|
|
||
Updated•22 years ago
|
|
|
||
Comment 16•22 years ago
|
||
> Put a <base href="http://www.aeg-hausgeraete.de/"> in every file and
> adjust the frameset in the main page to point to your local files.
In Mozilla trunk binary 2003042808 on WinNT, I can't run the original
HTML locally with these <base> tags added. I get errors in the JavaScript
Console like this:
Uncaught Exception: Permission denied to call Window.doMenu()
Uncaught Exception: Permission denied to call Window.fixMenu()
If I leave the <base> tags out, I can still see the long page-loading
time problem, even if the images don't all resolve properly.
zack-weg@gmx.de: can you say more about the <base> tags? Did you add
them to the parent frameset page as well as to each frame page?
What |src| did you use for the frames inside the frameset page?
Fully-qualified file:/// URLs, or relative URLs?
If I have a <base> tag at the top of the frameset page, and I use
relative URLS for the frames, Mozilla pulls the frames from
http://www.aeg-hausgeraete.de/ and gives me the above errors.
This is due to cross-domain security code in Mozilla: it refuses to
allow the frames off the Internet to call the functions in the parent
frameset page, which are in my local hard drive.
On the other hand, if I use fully-qualified file:/// URLs for the
frames, then <base> tags in each frame seem to confuse Mozilla.
I may be doing something wrong - the only thing that works for me
is to not use any <base> tags at all -
|
|
||
Comment 17•22 years ago
|
||
Sorry for the confusion, I've tested with a very old build (shouldn't do that,
I know), and behavior of security checks for docs with <base> has changed
(I consider the new behavior to be a bug, but it seems to be meant as a feature
-- at least the opposite is not true and a page cannot pretend to be local).
So indeed you cannot use <base> for the main page, but there is no need for it.
You can use an absolute URL for the third frame instead or just replace it with
about:blank, it's not important.
But while testing this I noticed that it's enough to change your testcase to
move the fixMenu() call in its own file (and of course remove the <base> too).
I'll set up a testcase.
|
|
||
Comment 18•22 years ago
|
||
|
|
||
Comment 19•22 years ago
|
||
|
|
||
Comment 20•22 years ago
|
||
|
|
||
Comment 21•22 years ago
|
||
zack-weg@gmx.de: thanks, you have solved the problem completely!!!
The attachment in Comment #19 takes a long time to load in Mozilla.
The attachment in Comment #20 does not. The only difference is the
wrapping of the global variables in Comment #20 to make them local.
Brilliant! This shows it's definitely the Mozilla DOM security checks
on global variables that slow the website down.
|
|
||
Comment 22•22 years ago
|
||
Another note: The two onload handlers contain a race condition. |fixMenu| calls
|doMenu| again if the other handler hasn't started to construct |arrMenuItems|
yet. And then they might construct |arrMenuItems| concurrently.
|
|
||
Comment 23•22 years ago
|
||
*** Bug 204529 has been marked as a duplicate of this bug. ***
|
||
Updated•16 years ago
|
Assignee: general → nobody
QA Contact: ashshbhatt → general
|
||
Comment 24•15 years ago
|
||
This is a very old bug, and the website that was a problem seems to have changed considerably since it was reported, but it loads quickly for me in Firefox 3.6 and so do all the testcases, so I'm closing this. Please reopen if it's still a problem for you, and give details.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•