1232814 - Sweep LazyScript in the background

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

Attachment: fix_NoteWeakEdge-v0.diff

Now that the weak reference to script is managed by WeakRef and not by ad-hoc barriers on access to the pointer, we can trivially sweep LazyScript in the background.

Or at least we should be able to. The first patch fixes NoteWeakReference. We were not actually early-returning in non-marking tracers, causing us to frob the weak refs set while out of the marking phase. Whoops!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=93d20dde3e00

Comment 1

[Terrence Cole [:terrence]]

Attachment: omt_LazyScript_finalize-v1.diff

Try is looking much less angry with the NoteWeakEdge fix.

Comment 2

[Jon Coppeard (:jonco)]

Comment on attachment 8698664 [details] [diff] [review]
omt_LazyScript_finalize-v1.diff

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

Great!

::: js/src/jsgc.cpp
@@ +393,5 @@
>      AllocKind::OBJECT_GROUP
>  };
>  
>  static const FinalizePhase BackgroundFinalizePhases[] = {
> +    PHASE(BackgroundPhaseScripts, gcstats::PHASE_SWEEP_SCRIPT),

For incremental finalization on the foreground thread, scripts are swept after strings.  I don't remember the reason for this but I think it makes sense to have a single order for both types of finalization.

Comment 3

[Terrence Cole [:terrence]]

D'oh! I thought I had made the order the same! Will fix it.

Comment 4

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8698662 [details] [diff] [review]
fix_NoteWeakEdge-v0.diff

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

Wow.

Uh... wow.

Comment 5

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5e94d0f7a3ddd3a81dd48332bb72fc8660be1e57
Bug 1232814 - Part 1: Fix a missing early return in NoteWeakEdge; r=sfink

https://hg.mozilla.org/integration/mozilla-inbound/rev/cdd02c0e74159bae2d7abc0de78907de479574aa
Bug 1232814 - Part 2: Move LazyScript finalization to the background finalization thread; r=jonco

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5e94d0f7a3dd
https://hg.mozilla.org/mozilla-central/rev/cdd02c0e7415