Closed Bug 708455 Opened 13 years ago Closed 12 years ago

IonMonkey: Discard JIT code before and after major GCs

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Attached patch fix (obsolete) — Splinter Review
This patch also adds some missing marking code.
Assignee: general → dvander
Status: NEW → ASSIGNED
Attachment #593701 - Flags: review?(christopher.leary)
Attached patch fix v2Splinter Review
Fixes bug in previous patch (we have to prevent tracing invalidated IonCode objects).
Attachment #593701 - Attachment is obsolete: true
Attachment #593701 - Flags: review?(christopher.leary)
Attachment #593707 - Flags: review?(christopher.leary)
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.
Attachment #593707 - Flags: review?(christopher.leary) → review+
(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.
http://hg.mozilla.org/projects/ionmonkey/rev/868eafc7deae
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 776353
You need to log in before you can comment on or make changes to this bug.