IonMonkey: Address worst-case compile performance on extremely large scripts

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dvander, Assigned: jandem)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Assignee: general → jdemooij
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Blocks: 758268
(Assignee)

Comment 1

5 years ago
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.
Attachment #627206 - Flags: review?(dvander)
(Reporter)

Updated

5 years ago
Attachment #627206 - Flags: review?(dvander) → review+
(Assignee)

Comment 2

5 years ago
http://hg.mozilla.org/projects/ionmonkey/rev/feac7727629c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
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
(Assignee)

Comment 4

5 years ago
(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.
You need to log in before you can comment on or make changes to this bug.