Closed Bug 785762 Opened 8 years ago Closed 8 years ago
Monkey: off thread code generation
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.
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+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.