Last Comment Bug 773655 - IonMonkey: Don't destroy JM code before Ion compilation
: IonMonkey: Don't destroy JM code before Ion compilation
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: ---
Assigned To: Jan de Mooij [:jandem]
: general
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: 771130
  Show dependency treegraph
Reported: 2012-07-13 08:13 PDT by Jan de Mooij [:jandem]
Modified: 2012-07-17 04:25 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (6.53 KB, patch)
2012-07-14 03:06 PDT, Jan de Mooij [:jandem]
no flags Details | Diff | Splinter Review
Patch (6.72 KB, patch)
2012-07-14 03:08 PDT, Jan de Mooij [:jandem]
bhackett1024: review+
Details | Diff | Splinter Review

Description User image Jan de Mooij [:jandem] 2012-07-13 08:13:07 PDT
Currently when a script is hot, we destroy the JM JITChunk, rejoin to the interpreter and enter Ion. Bug 771130 needs to access JM ICs from IonMonkey so we shouldn't destroy the JITChunk.
Comment 1 User image Jan de Mooij [:jandem] 2012-07-14 03:06:35 PDT
Created attachment 642202 [details] [diff] [review]

The main challenge is that if we don't destroy the chunk, JM callers may still call the function.

So when we want to enter Ion, the patch sets a flag to destroy the chunk later and NULLs out the JITScript entry pointers. This seemed to be the simplest and most effective way to prevent ICs from calling the function.
Comment 2 User image Jan de Mooij [:jandem] 2012-07-14 03:08:22 PDT
Created attachment 642203 [details] [diff] [review]

Forgot to qref.
Comment 3 User image Brian Hackett (:bhackett) 2012-07-14 03:19:09 PDT
Comment on attachment 642203 [details] [diff] [review]

Review of attachment 642203 [details] [diff] [review]:

::: js/src/methodjit/MethodJIT.h
@@ +783,5 @@
> +     * If set, we decided to keep the JITChunk so that Ion can access its caches.
> +     * The chunk has to be destroyed the next time the script runs in JM.
> +     * Note that this flag implies nchunks == 1.
> +     */
> +    bool mustDestroyChunk;

This name seems weird.  Maybe jit->mustDestroyEntryChunk or jit->mustDestroy?
Comment 4 User image Jan de Mooij [:jandem] 2012-07-17 04:25:42 PDT
Pushed with s/mustDestroyChunk/mustDestroyEntryChunk:

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