IonMonkey: Discard JIT code before and after major GCs

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
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.
(Assignee)

Comment 2

5 years ago
Created attachment 593701 [details] [diff] [review]
fix

This patch also adds some missing marking code.
Assignee: general → dvander
Status: NEW → ASSIGNED
Attachment #593701 - Flags: review?(christopher.leary)
(Assignee)

Comment 3

5 years ago
Created attachment 593707 [details] [diff] [review]
fix v2

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+
(Assignee)

Comment 5

5 years ago
(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.
(Assignee)

Comment 6

5 years ago
http://hg.mozilla.org/projects/ionmonkey/rev/868eafc7deae
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 776353
You need to log in before you can comment on or make changes to this bug.