621203 - IonMonkey: MD5 implementation slower than v8 and JSC

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: shell testcase

If I run the attached shell testcase, I get these numbers:

-m: 967
-j: 466
-m -j: 1165
-m -j -p: 988

So -p is helping over just -m -j, but it'd be nice to stay on trace altogether here...

Comment 1

[Boris Zbarsky [:bzbarsky]]

So we end up blacklisting 3 loops:

line 11: maybeShortLoop
line 159: isCompilationExpensive is true for the nested loop with 0 ops
line 48: selfOps hit 4096

Line 48 is the main md5 computation loop.  Line 159 is the driver loop.

It looks like isCompilationExpensive is still using >= for the "too branchy" check.  I thought we changed that to use >...

If I make that change, then we no longer claim to blacklist the loop on line 48, but we don't claim to trace it either.  I don't know what's going on there.  The time remains about the same.

I also just realized, btw, that the testcase is somewhat broken. I'll post a fixed version, but all of the above analysis still applies.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Requesting blocking, since this is a JM regression, btw.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fixed shell testcase

Comment 4

[Boris Zbarsky [:bzbarsky]]

In the fixed testcase, there's more going on, but the key part is blacklisting the line 48 loop because it has too many self ops...  But even if I bump the max op count to 16384 (in which case we're ok, because this thing only has about 6000 self ops), so that we decide to trace at line 48, I still see us being way slower with -m -j -p than with pure -j....  due to the fact that we blacklisted line 11 for being a maybe short loop and then didn't unblacklist it when we decided to trace the loop on 159.  Then we end up aborting over and over on the main driver loop due to not having a compatible tree on 11.

Bill, you have a bunch of profiler changes pending landing, right?  I suppose I should remeasure here once you land those....

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to comment #4)
> Bill, you have a bunch of profiler changes pending landing, right?  I suppose I
> should remeasure here once you land those....

Yeah, I want to get to that next week. I'll see how this bug fares after those land.

Comment 6

[David Mandelin [:dmandelin]]

I suggest that perf regressions should block at this point only if they affect a key benchmark or web site.

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Everything landed and this is still slow :-(. In fact, I see even worse numbers that Boris:

-m: 4863ms
-mj: 930ms
-mjp: 4613ms

Since it's not a blocker, I'm going to put this on hold.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Those numbers are about what I see on the "Fixed shell testcase" testcase.  The numbers in comment 0 were on the "shell testcase" testcase, which comment 1 describes as "somewhat broken".  ;)

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Current js shell results for the fixed testcase:
Interp: 73055
TM: 1949 (from 2011-10-01 nightly)
JM: 9565
JM+TI: 2803
d8: 4959

JM+TI is about 2x faster than v8. However, TM was about 1.4x faster than JM+TI. Changing the title to better reflect the current state of things.

Comment 10

[Till Schneidereit [:till]]

Current numbers on my rMBP@2.7Ghz:

SM: 2216 (with no arguments)
JSC: 1776
d8: 875

So, we're quite a bit slower than the competition, now. Changing the title to better reflect the current state of things.

Comment 11

[Till Schneidereit [:till]]

This is resoundingly fixed:

SM: 435
JSC: 1019
d8: 718