Closed Bug 838643 Opened 7 years ago Closed 7 years ago

GC: Finalize shapes on background thread


(Core :: JavaScript Engine, defect)

Not set





(Reporter: jonco, Assigned: jonco)




(1 file)

Currently shapes are finalized incrementally on the foreground thread.  If we can finalize them on the background thread, this potentially allows us to finalize some other types of objects on the background thread too, as their class information will be available from their shape.
Attached patch Possible fixSplinter Review
This patch incrementally sweeps the shape arenas in the foreground, unlinking dying shapes from the property tree.  It then finalizes them in the background where they actually get freed.  BaseShapes are also finalized in the background.

Try build here:

I had wanted to find a solution were I could do the unlinking just by traversing the live part of property tree rather than checking every shape cell, but I couldn't think of a way to do this incrementally.

Also it looks like sweepInitialShapeTable() discards entries if their associated prototype is dying, even if the shape is not.  We'd need to find a way to make sure we swept all live shapes for this to work.

I realised after doing this that Object::geClass() actually uses the object's type to get its class, not it's shape, so object finalization in the background doesn't immediately follow, but I think that can be worked around.
Attachment #710752 - Flags: review?(wmccloskey)
Comment on attachment 710752 [details] [diff] [review]
Possible fix

Review of attachment 710752 [details] [diff] [review]:

Thanks, this is great.

As a followup, it seems like we could just move TypeObject sweeping to the background. I don't know of any reason why that shouldn't work, and it would solve the getClass() issue.

::: js/src/jsgc.h
@@ +167,5 @@
>          false,     /* FINALIZE_OBJECT16 */
>          true,      /* FINALIZE_OBJECT16_BACKGROUND */
>          false,     /* FINALIZE_SCRIPT */
> +        true,      /* FINALIZE_SHAPE */
> +        true,     /* FINALIZE_BASE_SHAPE */

This should line up.
Attachment #710752 - Flags: review?(wmccloskey) → review+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Blocks: 843337
You need to log in before you can comment on or make changes to this bug.