Closed Bug 761618 Opened 7 years ago Closed 7 years ago

IonMonkey: Use IonMonkey only for hot scripts

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

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?
(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.
Attached patch Patch (obsolete) — Splinter Review
Attachment #630520 - Flags: review?(dvander)
Attached file Benchmark results (obsolete) —
SS/Kraken/V8 comparison
Attachment #630520 - Flags: review?(dvander)
Attached patch PatchSplinter Review
Attachment #630520 - Attachment is obsolete: true
Attachment #630525 - Flags: review?(dvander)
Attached file 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+
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
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.