626165 - Patch for bug 609212 doesn't entirely fix the "profiling makes us slower" thing

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

In-browser times with that patch for the testcase (linked in the url field):

-j: 515ms
-j -m: 520ms
-j -m -p: 640-700ms (very noisy)
-m: 1116ms

For comparison, Chrome 10 dev build is 644ms.  So on my machine we're about at parity with that; on the machine of the original reporter of bug 609212 (which is presumably 32-bit builds, since it's Windows; the above numbers for us are all 64-bit on Mac) we're about 16% slower than V8.  I don't know how much it matters that his machine is also slower than mine and may or may not have a bigger cache, etc.

Comment 1

[Bryan]

On the testcase for build: Mozilla/5.0 (Windows NT 5.1; rv:2.0b10pre) Gecko/20110115 Firefox/4.0b10pre ID:20110115102915 I get FPS: Processing: 1950ms

On the testcase for Chrome 8.0.552.237 I get FPS: Processing: 1691ms

~B

Comment 2

[Guilherme Lima]

Nightly 930-1100ms
Chrome 31 320-340ms
IE 10 850-910ms

Maybe the problem is 609296.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Maybe the problem is 609296.

It's possible.

A profile shows that on main thread about 1/3 of the time is under js::CallProperty called from jitcode.  Almost half of this is js::PrimitiveToObject.  Most of the rest is js::baseops::GetProperty, mostly under LookupPropertyWithFlagsInline.  Presumably this is some property being gotten on a primitive string that we don't manage to fast-path.

20% is js::SetObjectElement.  Most of this is JSObject::growElements and its realloc?
10% is js_num_toString.  
7% is js::jit::ArrayPushDense.
7% is str_toLowerCase.
5% is js::ConcatStrings
5% is CloneRegExpObject.
4% is str_replace
2% is js::jit::CheckOverRecursed
2% is js::jit::CharCodeAt

So the CallGetProperty and SetObjectElement are the main obvious issues.

Comment 4

[Jan de Mooij [:jandem]]

(Note that Peacekeeper no longer seems to have this test. Unfortunate because this test is a lot more reasonable than the other JS tests they have...)

I'll take a look though.

Comment 5

[Jan de Mooij [:jandem]]

The patch in bug 609296 improves this from 636 ms to 427 ms, that should take care of the CallGetProperty I think. Will look into the SetObjectElement calls now...

Comment 6

[Jan de Mooij [:jandem]]

The SetObjectElement calls we can fix by eagerly allocating this array: "new Array(80)". Bug 799122 did that for the most part, except when Array is inlined in Ion :(

That should get us to 325 ms or so, with bug 609296 almost 2x faster than trunk but still slower than d8. 

There's a bunch of smaller issues: Ion could inline number.toString(), with GGC MArrayPush can allocate elements directly without calling ArrayPushDense, etc.

Comment 7

[Jan de Mooij [:jandem]]

Filed bug 959161 for the first part of comment 6.

Comment 8

[Guilherme Lima]

Nightly for me now is 480-550ms... a 50% improvement in 2 months!

Comment 9

[April Caldwell]

Attachment: same problem

Thanks from tinkberheartter1978.