Last Comment Bug 761618 - IonMonkey: Use IonMonkey only for hot scripts
: IonMonkey: Use IonMonkey only for hot scripts
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Jan de Mooij [:jandem]
:
Mentors:
Depends on:
Blocks: IonSpeed
  Show dependency treegraph
 
Reported: 2012-06-05 08:07 PDT by Jan de Mooij [:jandem]
Modified: 2012-06-07 22:16 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (11.99 KB, patch)
2012-06-06 05:16 PDT, Jan de Mooij [:jandem]
no flags Details | Diff | Splinter Review
Benchmark results (6.19 KB, text/plain)
2012-06-06 05:17 PDT, Jan de Mooij [:jandem]
no flags Details
Patch (12.18 KB, patch)
2012-06-06 05:45 PDT, Jan de Mooij [:jandem]
dvander: review+
Details | Diff | Splinter Review
Benchmark results (6.15 KB, text/plain)
2012-06-06 05:45 PDT, Jan de Mooij [:jandem]
no flags Details

Description Jan de Mooij [:jandem] 2012-06-05 08:07:00 PDT
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.
Comment 1 David Anderson [:dvander] 2012-06-05 09:47:09 PDT
Sweet! What was the resolution re: JM to Ion calls?
Comment 2 Jan de Mooij [:jandem] 2012-06-05 11:25:57 PDT
(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.
Comment 3 Jan de Mooij [:jandem] 2012-06-06 05:16:37 PDT
Created attachment 630520 [details] [diff] [review]
Patch
Comment 4 Jan de Mooij [:jandem] 2012-06-06 05:17:20 PDT
Created attachment 630521 [details]
Benchmark results

SS/Kraken/V8 comparison
Comment 5 Jan de Mooij [:jandem] 2012-06-06 05:45:03 PDT
Created attachment 630525 [details] [diff] [review]
Patch
Comment 6 Jan de Mooij [:jandem] 2012-06-06 05:45:51 PDT
Created attachment 630528 [details]
Benchmark results
Comment 7 David Anderson [:dvander] 2012-06-06 09:09:32 PDT
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?
Comment 8 Jan de Mooij [:jandem] 2012-06-07 02:57:24 PDT
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.

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