Closed Bug 785762 Opened 7 years ago Closed 7 years ago

IonMonkey: off thread code generation

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: bhackett, Assigned: bhackett)

References

Details

(Whiteboard: [ion:t])

Attachments

(1 file)

The original patch in bug 774253 did code generation off thread, but the mechanism used was pretty clunky and this was cut from the final patch.  On my machine we currently spend about 4ms doing code generation on the main thread, so being able to do this phase on the worker thread would be nice.  I think dvander's suggestion in bug 774253 comment 14 is the best approach to use here, where the masm is filled in off thread but not linked to any of the lazy IonCompartment stubs, and the main thread does this linking, IonCode and executable code allocation when attaching the finished compilation.
Depends on: 786146
Attached patch patchSplinter Review
Linking turned out too complicated so building on top of bug 786146 instead.  This makes the remaining changes necessary to do everything in code generation off thread except allocate and fill in the final IonCode.
Assignee: general → bhackett1024
Attachment #655971 - Flags: review?(dvander)
Comment on attachment 655971 [details] [diff] [review]
patch

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

::: js/src/ion/CodeGenerator.cpp
@@ +2821,5 @@
>                             ? frameDepth_
>                             : FrameSizeClass::FromDepth(frameDepth_).frameSize();
>  
> +    unsigned slots = graph.localSlotCount() +
> +                     (graph.argumentSlotCount() * sizeof(Value) / STACK_SLOT_SIZE);

nit: if this is duplicated, we should make it a shared function.

::: js/src/ion/Ion.cpp
@@ +212,1 @@
>      Foreground::delete_(builder->temp().lifoAlloc());

Possible to hide these behind an IonBuilder::selfDestruct or something? So it's clear this frees the IonBuilder as well.

::: js/src/ion/IonBuilder.h
@@ +427,5 @@
>      types::RecompileInfo const recompileInfo;
>  
> +    // If off thread compilation is successful, final code is attached here.
> +    // This is heap allocated, and must be explicitly destroyed.
> +    CodeGenerator *codegen;

Nit: make this a private codegen_ and add a getter/setter, so the IonBuilder state isn't so exposed.
Attachment #655971 - Flags: review?(dvander) → review+
https://hg.mozilla.org/mozilla-central/rev/a5974ecf93c0
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Depends on: 810705
You need to log in before you can comment on or make changes to this bug.