Last Comment Bug 708455 - IonMonkey: Discard JIT code before and after major GCs
: IonMonkey: Discard JIT code before and after major GCs
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: David Anderson [:dvander]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: IonOSI 776353
Blocks: IM+TI
  Show dependency treegraph
 
Reported: 2011-12-07 15:52 PST by David Anderson [:dvander]
Modified: 2012-07-22 07:33 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (21.17 KB, patch)
2012-02-01 18:47 PST, David Anderson [:dvander]
no flags Details | Diff | Splinter Review
fix v2 (22.42 KB, patch)
2012-02-01 19:37 PST, David Anderson [:dvander]
cdleary: review+
Details | Diff | Splinter Review

Description David Anderson [:dvander] 2011-12-07 15:52:06 PST
This will simplify management of ICs and write barriers. IonMonkey is designed such that code can continue running during GC, though, so if this turn out to be bad for performance we can backtrack.

This should be trivial after on-stack invalidation lands, since we can just invalidate all frames.
Comment 1 David Anderson [:dvander] 2011-12-07 15:53:17 PST
Note, "discard JIT code" here is not in the JM since, where the JIT code is immediately freed. Invalidation causes us to return to the interpreter, but the JIT code is held live (IonCode objects are gcthings) until the stack unwinds.
Comment 2 David Anderson [:dvander] 2012-02-01 18:47:07 PST
Created attachment 593701 [details] [diff] [review]
fix

This patch also adds some missing marking code.
Comment 3 David Anderson [:dvander] 2012-02-01 19:37:34 PST
Created attachment 593707 [details] [diff] [review]
fix v2

Fixes bug in previous patch (we have to prevent tracing invalidated IonCode objects).
Comment 4 Chris Leary [:cdleary] (not checking bugmail) 2012-02-01 19:50:45 PST
Comment on attachment 593707 [details] [diff] [review]
fix v2

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

::: js/src/ion/Ion.cpp
@@ +1124,5 @@
> +
> +    script->ion->method()->setInvalidated();
> +
> +    /*
> +     * If this script has active Ion code, invalidation() will return

Nit: Can we say "Ion code on the stack" or "running Ion code" instead of "active Ion code"?

@@ +1128,5 @@
> +     * If this script has active Ion code, invalidation() will return
> +     * true. In this case we have to wait until destroying it.
> +     */
> +    if (!script->ion->invalidated())
> +        ion::IonScript::Destroy(cx, script->ion);

Nice.

::: js/src/ion/IonCode.h
@@ +320,5 @@
>      }
>      size_t size() const {
>          return safepointsStart_ + safepointsSize_;
>      }
> +    HeapValue &getConstant(size_t index) {

Why HeapValue for constants? Moving GC?

::: js/src/ion/shared/IonFrames-x86-shared.cpp
@@ -56,5 @@
>  #ifdef DEBUG
>      IonJSFrameLayout *frame = fp();
>      CalleeToken token = frame->calleeToken();
>      JS_ASSERT(token);
> -    JS_ASSERT(GetCalleeTokenTag(token) == CalleeToken_Function);

Thanks, nice catch.
Comment 5 David Anderson [:dvander] 2012-02-02 01:00:51 PST
(In reply to Chris Leary [:cdleary] from comment #4)
> >      }
> >      size_t size() const {
> >          return safepointsStart_ + safepointsSize_;
> >      }
> > +    HeapValue &getConstant(size_t index) {
> 
> Why HeapValue for constants? Moving GC?

Yeah.
Comment 6 David Anderson [:dvander] 2012-02-02 01:03:56 PST
http://hg.mozilla.org/projects/ionmonkey/rev/868eafc7deae

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