Closed Bug 896529 Opened 7 years ago Closed 7 years ago

IonMonkey: move some IonCompartment methods to IonRuntime

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: jandem, Assigned: jandem)

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
The shared IonCode stubs were moved to IonRuntime some time ago, but the methods to access them are still on IonCompartment. This is a bit confusing because it gives the impression the code is stored per compartment. This patch moves these methods to IonRuntime.
Attachment #779236 - Flags: review?(nicolas.b.pierron)
Comment on attachment 779236 [details] [diff] [review]
Patch

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

This sounds good as it is.

One small nit, and possibly a follow-up patch for removing the JSCompartment from the IonActivation, if this too big to be done as part of this patch.

::: js/src/ion/CodeGenerator.cpp
@@ +5434,5 @@
>      if (!generateArgumentsChecks())
>          return false;
>  
>      if (frameClass_ != FrameSizeClass::None()) {
> +        deoptTable_ = GetIonContext()->runtime->ionRuntime()->getBailoutTable(frameClass_);

Replace "GetIonContext()->runtime" by "gen".

::: js/src/ion/x86/Bailouts-x86.cpp
@@ +84,5 @@
>  
>      // Compute the snapshot offset from the bailout ID.
>      JitActivation *activation = activations.activation()->asJit();
> +    JSRuntime *rt = activation->compartment()->rt;
> +    IonCode *code = rt->ionRuntime()->getBailoutTable(bailout->frameClass());

From what I can grep in the code, the only use of activation->compartment() is to get the BailoutTable.  So I think we can just keep the IonRuntime pointer on the activation instead of the compartment.
Attachment #779236 - Flags: review?(nicolas.b.pierron) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/87571852c99f

(In reply to Nicolas B. Pierron [:nbp] from comment #1)
> Replace "GetIonContext()->runtime" by "gen".

Done.

> From what I can grep in the code, the only use of activation->compartment()
> is to get the BailoutTable.  So I think we can just keep the IonRuntime
> pointer on the activation instead of the compartment.

JitActivation is a subclass of Activation. Activation::compartment_ is used by ScriptFrameIter::compartment, JSCompartment::hasScriptsOnStack() and probably others.
https://hg.mozilla.org/mozilla-central/rev/87571852c99f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.