Closed
Bug 386771
Opened 18 years ago
Closed 12 years ago
JS Performance Improvements
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 579390
People
(Reporter: mtschrep, Unassigned)
References
()
Details
Attachments
(1 file)
5.61 KB,
text/html
|
Details |
Running uBenchmarks hosted at above site show some interesting numbers:
IE7:
Random number engine 129
Math engine 297
Array functions 49
String functions 34
Ajax declaration 3780 (this test is old and is using ActiveXObject)
Minefield (7/3/07):
Try/Catch with errors 60
Random number engine 251
Math engine 375
Array functions 107
String functions 72
Ajax declaration 67
Safari 3.0.2 Beta:
Try/Catch with errors 8
Random number engine 66
Math engine 115
Array functions 113
String functions 20
Ajax declaration 19
I've filed separate bugs on the other tests which test DOM interactions - these look to be completely in JS. The Try/Catch test looks questionable because people should not be throwing exceptions in tight loops. Math, Array, String, and maybe Random however seem like they'd be much more common operations. Given the delta in numbers here this looks like a ripe area for optimization.
Comment 1•18 years ago
|
||
The Array test is pretty unrealistic: it converts keys to strings by failing to pass a numeric comparison function to .sort() (we have a bug on optimizing this, hence the DUPEME); it reverses each time, so all but the first sort calls are dealing with mostly-reverse-ordered data, which is uncommon on the web.
The String test may be measuring GC, not string operations. It's hard to say at a glance but a breakpoint in js_GC during its run, and some timing numbers when the loop count is lowered to avoid GC, would tell.
There are other bugs on file covering these areas better, and there may even be a bug on this site's benchmarks. More DUPEME fodder. It's ok if this bug stays around to track the site as a whole, but it should be a metabug, and probably just a reminder-dependency of an existing JS perf metabug. Bob, can you suggest some candidate metabugs?
/be
Whiteboard: DUPEME
Comment 2•18 years ago
|
||
The most basic meta bug is Bug 117611 which I've just updated with the open jseng bugs with perf keywords. I'll look for additional ones this evening.
Reporter | ||
Comment 3•18 years ago
|
||
Here's a slightly modified set of array specific tests:
1) As before
2) Addition of a compare function to sort
3) Build the array in a loop and do one big sort
Results below. Interestingly the comparison function causes both IE and Safari to do worse (do they do specific optimizations otherwise that get disabled)? If we can detect the string->int case and get similar perf to when a comparison function is provided we'll meet or best IE7.
Minefield:
Array functions 64
Array functions with int comparator 43
Array functions one big sort 10
Total Duration 117ms
Safari:
Array functions 116
Array functions with int comparator 153
Array functions one big sort 29
Total Duration 298 ms
IE7:
Array functions 47
Array functions with int comparator 541
Array functions one big sort 135
Total Duration 723ms
Reporter | ||
Comment 4•18 years ago
|
||
Any thoughts on Math.Random() - not clear that's something people would do in a tight loop but unsure whether this is some generic call overhead to tackle here.
Comment 5•18 years ago
|
||
(In reply to comment #3)
> If we can detect the string->int
(int->string)
> case and get similar perf to when a comparison
> function is provided we'll meet or best IE7.
See bug 371636 comment 6. I think that bug 371636 should be morphed into a bug requesting that we optimize this default-comparator case to beat IE.
(In reply to comment #4)
> Any thoughts on Math.Random() - not clear that's something people would do in a
> tight loop but unsure whether this is some generic call overhead to tackle
> here.
A null method benchmark (null native and null scripted benchmarks) would show that. This is probably irrelevant, since Math.random should be higher quality, not higher performance, than it has been, without being "crypto grade". Does the benchmark for Math.random even try to assess PRNG quality? I didn't look at that one yet.
/be
Comment 6•18 years ago
|
||
See bug 322529 for Math.random quality work that will probably slow it down a bit (not that that matters ;-).
I morphed the summary of bug 371636 to talk about default comparator and numbers. The patch there never had a review request flag set; I'll ask for a fresh patch.
/be
Reporter | ||
Comment 7•18 years ago
|
||
> See bug 371636 comment 6. I think that bug 371636 should be morphed into a bug
> requesting that we optimize this default-comparator case to beat IE.
Awesome!
> A null method benchmark (null native and null scripted benchmarks) would show
> that. This is probably irrelevant, since Math.random should be higher quality,
> not higher performance, than it has been, without being "crypto grade". Does
> the benchmark for Math.random even try to assess PRNG quality? I didn't look at
> that one yet.
Umm. Nope. The entire thing is:
for( var i = 0; i <= 50000; i++ )
{
var a = Math.random()
}
Not a lot of assessment of much of anything in there :-).
Comment 8•18 years ago
|
||
IE7 does poorly on js1_5/Regress/regress-211590.js with 38.49 and 25.2 while Safari does better but still "fails" with 41.5, so the answer is they don't care about the quality of the answer.
Comment 9•18 years ago
|
||
Bob, thanks for reminding me about bug 211590. That was a good contribution from zack-weg@gmx.de (cc'ing him here in case he's still active). I repeat that there is no point in making Math.random really fast at low or zero quality. It's not on any performance-critical paths I've ever seen, but its quality definitely matters to some real-world JS I've seen.
/be
Comment 10•18 years ago
|
||
What the Random Number and Math engine test really demonstrate is bug 169559, if you make Math a local var in those functions you'll get much better results.
Quite frankly, I'm baffled by the attention this badly written benchmark gets. The flaws in the try-catch and array benchmarks have been pointed out already but what about that string test? It doesn't actually call toUpperCase and toLowerCase...
How sloppy must a benchmark be before you dismiss its results as irrelevant?
Comment 11•18 years ago
|
||
Any fool with a browser and a blog can make up and try to popularize synthetic bogo-benchmarks, for sure. We do not want to optimize for such benchmarks, ever. The strongtalk page on benchmarking is worth reading or re-reading:
http://strongtalk.org/benchmarking.html
Still, we should have a way of separating wheat from chaff where there's enough wheat to be profitable. This bug may lead to a few fixes, even if the real bugs are already on file. Also we can spread knowledge of how to benchmark well.
/be
Comment 12•18 years ago
|
||
Quick followup: I don't mean to call whoever put up celtickane.com's benchmarks a fool (as in "any fool") -- they don't look good from what I've seen, but it's easy to make mistakes when writing "my first benchmark". I do mean that "anyone" even if inexperienced or unqualified can popularize silly JS benchmarks. I've seen 'em for almost 12 years now.
Seno, the new form of "global code is slower than local due to variable access" bug seems to be bug 376291. It should be investigated and fixed for 1.9. Any help you can give there would be appreciated.
/be
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•