1377466 - Reconsider the JS::PokeGC call in XPCRootSetElem::RemoveFromRootSet()

Status: RESOLVED WORKSFORME

(Core :: JavaScript: GC, enhancement)

Description

[Andrew McCreight [:mccr8]]

XPCRootSetElem::RemoveFromRootSet() is called when we destroy various XPConnect data structures that hold alive JS objects. This calls JS::PokeGC, which sets a flag on the JS runtime. (This is the only call to PokeGC.) If that flag is set, and we're in a shutdown or compacting GC, then we do a full GC. This also causes us to do a second GC.

Note that this does not affect the GC reason, so I don't think we have any telemetry on how often this happens.

It would be nice if we could just schedule a GC for the chrome zone, rather than a full GC. I don't know if XPCWrappedJS can wrap content JS or not. I vaguely remember some horrible reasons we do this poke flag thing, though so maybe not.

We may at least want to collect some telemetry on how often the poke flag is affecting the GC.

Comment 1

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed043698eafa
Rename PokeGC and add a reason code for GCs triggered by this r=sfink

Comment 2

[Andrew McCreight [:mccr8]]

Comment 1 landed with the wrong bug number.

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ed043698eafa

Comment 4

[Jon Coppeard (:jonco)]

Sorry, this was the fix for bug 1380025.

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Andrew McCreight [:mccr8] from comment #0)
We now have a separate reason code for this (bug 1380025) and it now only happens in shutdown GC (bug 1380387).  

The 'poke' is still necessary to ensure that everything is collected in a shutdown GC.

Please reopen if you think there's more to do here.