IonMonkey: Use IonMonkey only for hot scripts

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Currently JM and Ion use the same strategy to decide when a script is compiled:

1) Initial compilation after 40 calls and/or loop backedges.
2) Recompilation after 10,000 calls/backedges to inline calls.

These numbers make sense for JM because the alternative (interpreting) is very slow and compilation is pretty fast.

For Ion, the plan is to:

1) JM compilation after 40 calls/backedges.
2) Ion compilation after 10,000 calls/backedges, inlining calls.

The main benefit is that we don't waste time optimizing "warm", not hot, functions and this avoids some regressions, especially on SS.

I prototyped this change and the initial results are promising, SS/Kraken/V8 all become faster.
Sweet! What was the resolution re: JM to Ion calls?
(Assignee)

Comment 2

5 years ago
(In reply to David Anderson [:dvander] from comment #1)
> Sweet! What was the resolution re: JM to Ion calls?

It uses some simple heuristics (just a few lines of code), e.g. perform a JM -> Ion call only if both caller and callee are Ion-compileable, the callee has at least one loop and the caller is not (very) hot. In the other cases we fallback to JM for the callee. I tried some other things (transitioning to Ion after the script prologue) but these rules are pretty simple (I will explain them in the patch) and gave the best results.
(Assignee)

Comment 3

5 years ago
Created attachment 630520 [details] [diff] [review]
Patch
Attachment #630520 - Flags: review?(dvander)
(Assignee)

Comment 4

5 years ago
Created attachment 630521 [details]
Benchmark results

SS/Kraken/V8 comparison
(Assignee)

Updated

5 years ago
Attachment #630520 - Flags: review?(dvander)
(Assignee)

Comment 5

5 years ago
Created attachment 630525 [details] [diff] [review]
Patch
Attachment #630520 - Attachment is obsolete: true
Attachment #630525 - Flags: review?(dvander)
(Assignee)

Comment 6

5 years ago
Created attachment 630528 [details]
Benchmark results
Attachment #630521 - Attachment is obsolete: true
Comment on attachment 630525 [details] [diff] [review]
Patch

Review of attachment 630525 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/methodjit/Compiler.cpp
@@ +48,5 @@
>      JS_END_MACRO
>  
>  /*
>   * Number of times a script must be called or had a backedge before we try to
> + * inline its calls. This number should match IonMonkey's usesBeforeCompile.

Is it worth hoisting this constant out?
Attachment #630525 - Flags: review?(dvander) → review+
(Assignee)

Comment 8

5 years ago
http://hg.mozilla.org/projects/ionmonkey/rev/8acbac4d6cfd

(In reply to David Anderson [:dvander] from comment #7)
> 
> Is it worth hoisting this constant out?

I considered it but I couldn't think of a good place to put it: both JITs use the constant even if the other JIT is disabled. Maybe jsscript.h but that also seemed a bit confusing.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.