Closed Bug 376291 Opened 18 years ago Closed 17 years ago

Inline javascript code dramatically slower than the same code placed in a function

Categories

(Core :: JavaScript Engine, defect)

x86
Windows XP
defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: duncan.loveday, Assigned: crowderbt)

References

Details

(Keywords: perf, regression)

Attachments

(4 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a4pre) Gecko/20070331 Minefield/3.0a4pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a4pre) Gecko/20070331 Minefield/3.0a4pre

A simple, processor-bound javascript loop executes orders of magnitude more slowly in an inlined script as compared to the same loop placed in a function and called from an inlined script.

Reproducible: Always

Steps to Reproduce:
1. Open the attached test case in Firefox
2.
3.
Actual Results:  
My results for FF3 are
Inlined, in <head>          5735ms; Function, in <head>         109ms
Inlined, in <body>          5766ms; Function, in <body>         109ms
Inlined, after <body>       5703ms; Function, after <body>      94ms

Expected Results:  
The inlined and non-inlined timings should be about the same.

I don't know whether this is important, but it is strange and unexpected. Can one be assured that the javascript engine will always run things at the faster speed when functions are used, or might the underlying cause sometimes affect code in a function ?

Out of interest, my results for FF2 are even stranger since there they vary depending on whether or not the code is placed in the <head> or not. My results for FF2 are
Inlined, in <head>          3156ms; Function, in <head>         125ms
Inlined, in <body>          547ms; Function, in <body>          125ms
Inlined, after <body>       547ms; Function, after <body>       125ms
Attached file test HTML source (deleted) —
Marked this as a regression since the slowdown is rather more significant on FF3.
Keywords: perf, regression
This is happening because your loop variable "n" is a global variable in the first, third, and fifth test cases, and so its lookup time is dramatically slower.  If you hoisted the loop variable out of the function in tests 2, 4, and 6, you would likely get equally bad results, or worse.  That is your primary cost, here, I think.  This Opera page contains a discussion on the issue:

http://dev.opera.com/articles/view/efficient-javascript/?page=2#avoidglobal

There may be hacks or reasonable heuristics for fixing performance issues like this, but I don't think they're likely to be undertaken any time soon.  We should either WONTFIX this, or morph it to a bug on access to globals having slowed down on the trunk.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I'll investigate the morphed version of this.
Assignee: general → crowder
Summary: Inline javascript code dramatically slower than the same code placed in a function → global access slower on trunk than FF2
Status: NEW → ASSIGNED
Summary: global access slower on trunk than FF2 → JavaScript: global variable access slower on trunk than FF2
Brian, access to globals is definitely part of the story but not the whole story. My results using globals throughout are

My results for FF3 using globals throughout are
Inlined, in <head>          5937ms; Function, in <head>         766ms
Inlined, in <body>          5891ms; Function, in <body>         781ms
Inlined, after <body>       5860ms; Function, after <body>      765ms
Attached file Using global loop variable throughout (deleted) —
Restoring the original summary, and will investigate further.
Summary: JavaScript: global variable access slower on trunk than FF2 → Inline javascript code dramatically slower than the same code placed in a function
Here's a shark profile of just libmozjs during the run of this script:

47.0%	47.0%	libmozjs.dylib	js_NativeGet
25.7%	25.7%	libmozjs.dylib	js_NativeSet
7.1%	7.1%	libmozjs.dylib	js_Interpret
4.7%	4.7%	libmozjs.dylib	js_SearchScope
3.0%	3.0%	libmozjs.dylib	js_LookupPropertyWithFlags
2.5%	2.5%	libmozjs.dylib	JS_GetPrivate
2.0%	2.0%	libmozjs.dylib	JS_GetClass
1.5%	1.5%	libmozjs.dylib	JS_FrameIterator
1.4%	1.4%	libmozjs.dylib	js_FindProperty
1.2%	1.2%	libmozjs.dylib	js_GetProperty
0.9%	0.9%	libmozjs.dylib	js_SetProperty
0.8%	0.8%	libmozjs.dylib	js_LookupProperty
0.6%	0.6%	libmozjs.dylib	JS_EndRequest
0.4%	0.4%	libmozjs.dylib	JS_GetFrameFunctionObject
0.3%	0.3%	libmozjs.dylib	JS_BeginRequest
0.2%	0.2%	libmozjs.dylib	JS_GetFrameScript
0.2%	0.2%	libmozjs.dylib	js_DropProperty
0.2%	0.2%	libmozjs.dylib	JS_TypeOfValue
0.2%	0.2%	libmozjs.dylib	JS_GetScriptPrincipals
0.1%	0.1%	libmozjs.dylib	JS_GetContextPrivate
0.0%	0.0%	libmozjs.dylib	JS_HashTableRawLookup
0.0%	0.0%	libmozjs.dylib	js_HoldObjectMap
0.0%	0.0%	libmozjs.dylib	JS_ArenaAllocate

The bulk of js_NativeGet()'s time is spent in the sprop->getter portion of SPROP_GET (97.1%), which is a call to XPC_WN_Helper_GetProperty:

0.4%	23.8%	libxpconnect.dylib XPC_WN_Helper_GetProperty
0.9%	21.7%	libgklayout.dylib   nsWindowSH::GetProperty
0.5%	16.6%	libgklayout.dylib    nsDOMClassInfo::doCheckPropertyAccess
0.3%	13.7%	libcaps.dylib         nsScriptSecurityManager::CheckPropertyAccess

Which seems to indicate we're leaving the happy confines of JS to do security checks we really don't need to do.  I'm not even really clear on why this XPC_WN_Helper_GetProperty is happening so frequently.

Anyone have any thoughts or hints on this?
dveditz, you might be interested in comment 8.
It would be interesting to compare profiles of the test code executing inline vs in a function to see what's different.
I wonder if this is a result of bug 339918. Because of that bug, we'll have to do security checks for each window/document property access in an inline script, but not for each access in a function.
So perhaps bug 367911 will help.
Depends on: xow
Attached file Two loops not in functions (deleted) —
It seems what's making the difference here is calling a function before the loop, not whether the loop is inside a function. If you look at the testcase you'll see that it's the same loop twice, just with a call to 'storeResults' inbetween. If that call happens first then the first loop is just as fast as the second.
The last build where this was working correctly is 2006-06-05-05 and indeed the patch for bug 339918 was checked in on that day.
mrbkap said he might help crowder on this one -- he knows the inside baseball and it's non-trivial.

Seno, can you test with n declared using var and report the speedup?

/be
Well, just declaring n as a global variable doesn't make a difference. Using 'let' to make n a block-local variable makes the loop as fast as it would be in a function with a function-local variable. To give you an idea of the magnitude of the speed differences:

global n, this bug in effect: 3907ms
global n, this bug not in effect: 625ms.
block-local n: 94ms

So global variables are almost 7 times slower in the best case and this bug slows them down another ~650%.
Seno, would you attach your latest testcase?
Attached file testcase with block-scoped n (deleted) —
Here you go. I've now found that all you need to do to speed up the script is call a function that accesses a global var. For example, if I start the script with
   (function () { isNaN }())
then the loop runs with the normal speed.
I think this should now be fixed. The offending code has been removed entirely.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Results using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007073005 Minefield/3.0a7pre using globals throughout are

Inlined, in <head>          839(5937)ms; Function, in <head>         800(766)ms
Inlined, in <body>          817(5891)ms; Function, in <body>         803(781)ms
Inlined, after <body>       817(5860)ms; Function, after <body>      823(765)ms

Old numbers in brackets. So yes, this seems to be fixed.
Removed as part of bug 367911 (cross-origin wrappers) or removed elsewhere?
Removed as part of bug 367911.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: