386771 - JS Performance Improvements

Status: RESOLVED DUPLICATE of bug 579390

(Core :: JavaScript Engine, defect)

Description

[Mike Schroepfer]

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

[Brendan Eich [:brendan]]

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

Comment 2

[Bob Clary [:bc] (inactive)]

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.

Comment 3

[Mike Schroepfer]

Attachment: Single-File Test for Local comparisons

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

Comment 4

[Mike Schroepfer]

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

[Brendan Eich [:brendan]]

(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

[Brendan Eich [:brendan]]

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

Comment 7

[Mike Schroepfer]

> 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

[Bob Clary [:bc] (inactive)]

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

[Brendan Eich [:brendan]]

Bob, thanks for reminding me about bug 211590. That was a good contribution from [e-mail] (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

[Aiko]

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

[Brendan Eich [:brendan]]

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

[Brendan Eich [:brendan]]

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