Last Comment Bug 755010 - IonMonkey: Address worst-case compile performance on extremely large scripts
: IonMonkey: Address worst-case compile performance on extremely large scripts
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Jan de Mooij [:jandem]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: LandIon 758268
  Show dependency treegraph
 
Reported: 2012-05-14 13:40 PDT by David Anderson [:dvander]
Modified: 2012-05-26 02:45 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (5.08 KB, patch)
2012-05-25 06:00 PDT, Jan de Mooij [:jandem]
dvander: review+
Details | Diff | Splinter Review

Description David Anderson [:dvander] 2012-05-14 13:40:56 PDT
We need some basic heuristics for deciding whether a script is too large to compile with IonMonkey.

First, local and argument counts should be capped to something relatively small (probably 256 is okay, 512 might be pushing it - we can measure).

Second, if a JSScript gets recompiled too many times, we should stop trying to optimize it (unless useCount gets reset).

Last, we should have some heuristic for detecting when a function is super expensive to compile, because then if it recompiles at all we'll probably lose.

In all of these cases for now we can fall back to JM+TI.
Comment 1 Jan de Mooij [:jandem] 2012-05-25 06:00:04 PDT
Created attachment 627206 [details] [diff] [review]
Patch

This patch caps script->length and the number of locals + args. It seems to work pretty well.

On Kraken, this helps audio-beat-detection, other tests are unaffected:

    beat-detection: 2.18x as fast      435.6ms +/- 2.9%    200.2ms +/-

V8 before:

Richards: 9761
DeltaBlue: 8179
Crypto: 13756
RayTrace: 977
EarleyBoyer: 2507
RegExp: 495
Splay: 9298
----
Score (version 6): 3843

V8 after:

Richards: 9779
DeltaBlue: 8106
Crypto: 13868
RayTrace: 969
EarleyBoyer: 2428
RegExp: 1202
Splay: 9420
----
Score (version 6): 4347

SS score does not change much, although we de do reject the large scripts in 3d-cube and crypto-m5. 3d-cube gets 3 ms faster and md5 1 ms slower. Both are still slower than JM+TI, I will look into this now.
Comment 2 Jan de Mooij [:jandem] 2012-05-25 09:38:31 PDT
http://hg.mozilla.org/projects/ionmonkey/rev/feac7727629c
Comment 3 Nicolas B. Pierron [:nbp] 2012-05-25 10:40:49 PDT
What is the impact on emscripten generated code ?  misc-bugs-654410-nezulator[1] seems to suffer a lot from this modification.

[1] http://arewefastyet.com/?machine=9&view=assorted
Comment 4 Jan de Mooij [:jandem] 2012-05-26 02:45:59 PDT
(In reply to Nicolas B. Pierron [:pierron] from comment #3)
> What is the impact on emscripten generated code ? 

I tested sqlite.js and it runs fine now.

> misc-bugs-654410-nezulator[1] seems to suffer a lot from this modification.

I forgot to mention this, but the AWFY regressions are expected: we now reject large scripts and JM is disabled on AWFY. Enabling JM should fix the regressions.

Note You need to log in before you can comment on or make changes to this bug.